RE: [PATCH] driver core: device: add BUS_ATTR_WO macro
> > Add BUS_ATTR_WO macro to make it easier to add attributes without > > auditing the mode settings. Also, use the newly added macro where > > appropriate. > > > > Signed-off-by: Ioana Ciornei > > --- > > arch/powerpc/platforms/pseries/ibmebus.c | 12 > > drivers/block/rbd.c | 48 > > > > drivers/scsi/fcoe/fcoe_sysfs.c | 4 +-- > > drivers/scsi/fcoe/fcoe_transport.c | 10 +++ > > include/linux/device.h | 2 ++ > > include/scsi/libfcoe.h | 8 +++--- > > 6 files changed, 43 insertions(+), 41 deletions(-) > > Nice! This duplicates a lot of the work I did back in July but have not > pushed out > very far due to the other things that ended up happening around that time: > > https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git/log/?h=bus_cleanup > > > As the patch series seen at that link shows, you can do this in more places > than > just what you did here. > > Either way, you should break this up into the individual patches, like I did > or you > can take my patches if you want. Getting the BUS_ATTR_WO() macro added is > good to do now, and then you can go and hit up all of the different subsystems > that should be converted over to it. I can of course split my patch into individual ones and resubmit them, but as you already have the entire patch set ready, I feel like we can just push those. I looked through your changes and it seems like you covered all the subsystems. Please let me know if there is something else I should do. My intention here was to first add the _WO attribute so that afterwards I can add a new bus attribute in the fsl-mc bus. Ioana > > thanks, > > greg k-h
Re: [PATCH] scsi: mptfusion: Remove unnecessary parentheses and simplify null checks
On Sat, Sep 15, 2018 at 8:38 AM Nathan Chancellor wrote: > Reported-by: Nick Desaulniers > Signed-off-by: Nathan Chancellor > --- > drivers/message/fusion/mptbase.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/message/fusion/mptbase.c > b/drivers/message/fusion/mptbase.c > index dc1e43a02599..ba551d8dfba4 100644 > --- a/drivers/message/fusion/mptbase.c > +++ b/drivers/message/fusion/mptbase.c > @@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg) > MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg; > struct pci_dev *pdev; > > - if ((ioc == NULL)) > + if (!ioc) > return -1; > > pdev = ioc->pcidev; > - if ((pdev == NULL)) > + if (!pdev) > return -1; > > pci_stop_and_remove_bus_device_locked(pdev); > -- The exact same code also exists in drivers/scsi/mpt3sas/. I had a similar patch for both drivers, but saw that yours now got merged for fusion. If you don't mind, could you fix mpt3sas_base.c the same way? Arnd
Re: [PATCH v4 1/6] mpt3sas: Introduce mpt3sas_base_pci_device_is_available
On Mon, Oct 01, 2018 at 03:40:51PM -0500, Bjorn Helgaas wrote: > I think the names "pci_device_is_present()" and > "mpt3sas_base_pci_device_is_available()" contribute to the problem > because they make promises that can't be kept -- all we can say is > that the device *was* present, but we know whether it is *still* > present. Oops, I meant "we DON'T know whether it is still present." > I think it would be better if the interfaces were something > like "pci_device_is_absent()" because that gives a result we can rely > on. If that returns true, we know the device is definitely gone. > > Bjorn
Re: [PATCH] libosd: Remove ignored __weak attribute
On Mon, Oct 01, 2018 at 06:16:32PM -0700, Bart Van Assche wrote:
> Boaz, the most recent osd patch that is neither trivial nor treewide
> refactoring is six years old (51976a8c85ce ("[SCSI] osd_uld: Add osdname &
> systemid sysfs at scsi_osd class"). That suggests that nobody is using this
> driver anymore. Can this driver be removed from the kernel tree?
Last time we tried to exofs and the osd code Boaz complained loudly,
but as far as I can tell it really is not used and bitrotting, so we
should finally go ahead and drop it.
Re: [PATCH] MAINTAINERS: Fix typo in cxlflash stanza
> On Oct 1, 2018, at 11:26 AM, Matthew R. Ochs wrote: > > The uapi header file listed in the cxlflash stanza has a typo. > > Removed the trailing 's' from the filename. > > Reported-by: Joe Perches > Signed-off-by: Matthew R. Ochs > --- Thanks for taking care of this. Acked-by: Uma Krishnan
Re: [PATCH] scsi: mptfusion: Remove unnecessary parentheses and simplify null checks
On Tue, Oct 02, 2018 at 02:58:21PM +0200, Arnd Bergmann wrote: > On Sat, Sep 15, 2018 at 8:38 AM Nathan Chancellor > wrote: > > > Reported-by: Nick Desaulniers > > Signed-off-by: Nathan Chancellor > > --- > > drivers/message/fusion/mptbase.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/message/fusion/mptbase.c > > b/drivers/message/fusion/mptbase.c > > index dc1e43a02599..ba551d8dfba4 100644 > > --- a/drivers/message/fusion/mptbase.c > > +++ b/drivers/message/fusion/mptbase.c > > @@ -335,11 +335,11 @@ static int mpt_remove_dead_ioc_func(void *arg) > > MPT_ADAPTER *ioc = (MPT_ADAPTER *)arg; > > struct pci_dev *pdev; > > > > - if ((ioc == NULL)) > > + if (!ioc) > > return -1; > > > > pdev = ioc->pcidev; > > - if ((pdev == NULL)) > > + if (!pdev) > > return -1; > > > > pci_stop_and_remove_bus_device_locked(pdev); > > -- > > > The exact same code also exists in drivers/scsi/mpt3sas/. I had a similar > patch for both drivers, but saw that yours now got merged for fusion. > > If you don't mind, could you fix mpt3sas_base.c the same way? > > Arnd Hi Arnd, I did sent a patch for this a little bit later: https://lore.kernel.org/lkml/20180920201002.23979-1-natechancel...@gmail.com/ I am still waiting for it to be merged. Thanks! Nathan
Re: [PATCH RESEND] scsi: sg: Prevent potential double frees in sg driver
On Mon, Oct 1, 2018 at 4:34 PM Douglas Gilbert wrote: > > On 2018-10-02 02:15 AM, Evan Green wrote: > > From: Robb Glasser > > > > sg_ioctl could be spammed by requests, leading to a double free in > > __free_pages. This protects the entry points of sg_ioctl where the > > memory could be corrupted by a double call to __free_pages if multiple > > requests are happening concurrently. > > Hi, > I don't like this patch. I would like to see the trace for the double > call to the __free_pages you are referring too. A test program that > show the fault, perhaps? > > I have test code to "spam" the sg driver and have not seen a double > __free_pages that you refer to (see sg3_utils package version 1.44, > testing/sg_tst_async.cpp). > > Currently I am dusting off 20 years of "laparoscopic" patches to the sg > driver that have made a bit of a mess of the naming and comments. Also > the 16 outstanding requests per file descriptor limit is being removed. > Then I want to add the SG_IOSUBMIT and SG_IORECEIVE ioctls proposed by > Linus Torvalds two week ago. > > Executive summary: nak, without further information That makes sense. Thanks for taking a look.
Re: [PATCH] libosd: Remove ignored __weak attribute
On 02/10/18 17:56, Christoph Hellwig wrote:
> On Mon, Oct 01, 2018 at 06:16:32PM -0700, Bart Van Assche wrote:
>> Boaz, the most recent osd patch that is neither trivial nor treewide
>> refactoring is six years old (51976a8c85ce ("[SCSI] osd_uld: Add osdname &
>> systemid sysfs at scsi_osd class"). That suggests that nobody is using this
>> driver anymore. Can this driver be removed from the kernel tree?
>
> Last time we tried to exofs and the osd code Boaz complained loudly,
> but as far as I can tell it really is not used and bitrotting, so we
> should finally go ahead and drop it.
>
As I said then. It is used in Universities for studies and experiments.
Every once in a while. I get an email with questions and reports.
But yes feel free to remove the all thing!!
I guess I can put it up on github. In a public tree.
Just that I will need to forward port it myself, til now you guys
been doing this for me ;-)
Thanks, sorry for the hassle
Boaz
Re: [PATCH] libosd: Remove ignored __weak attribute
On Mon, Oct 1, 2018 at 6:16 PM Bart Van Assche wrote:
>
> On 9/30/18 1:54 PM, Nathan Chancellor wrote:
> > diff --git a/include/scsi/osd_types.h b/include/scsi/osd_types.h
> > index 48e8a165e136..6b6fdcafa6cc 100644
> > --- a/include/scsi/osd_types.h
> > +++ b/include/scsi/osd_types.h
> > @@ -28,7 +28,7 @@ struct osd_obj_id {
> > osd_id id;
> > };
> >
> > -static const struct __weak osd_obj_id osd_root_object = {0, 0};
> > +static const struct osd_obj_id osd_root_object = {0, 0};
>
> Structure definitions should occur in .c files instead of in header
> files especially if the header file is included from multiple source
> files. Please consider moving the definition of osd_root_object into a
> .c file.
> Additionally, zero initializers should be left out to minimize
> the size of object files.
Sorry, my understanding was that global variables either occupy the
.bss section or the .data section, depending on whether they were
zero-initialized vs initialized to non-zero, respectively (where
non-initialized are treated as zero initialized). Looks like without
the explicit zero initialization, compilers will put the symbols in a
"common" section, which `man 1 nm` says is also unitialized data. I
didn't think .bss sections occupied space in an object file or binary;
the kernel's loader would set up the mappings at execution? Can you
clarify?
>
> Boaz, the most recent osd patch that is neither trivial nor treewide
> refactoring is six years old (51976a8c85ce ("[SCSI] osd_uld: Add osdname
> & systemid sysfs at scsi_osd class"). That suggests that nobody is using
> this driver anymore. Can this driver be removed from the kernel tree?
>
> Thanks,
>
> Bart.
--
Thanks,
~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Tue, 2018-10-02 at 10:24 -0700, Nick Desaulniers wrote:
> On Mon, Oct 1, 2018 at 6:16 PM Bart Van Assche wrote:
> > Additionally, zero initializers should be left out to minimize
> > the size of object files.
>
> Sorry, my understanding was that global variables either occupy the
> .bss section or the .data section, depending on whether they were
> zero-initialized vs initialized to non-zero, respectively (where
> non-initialized are treated as zero initialized). Looks like without
> the explicit zero initialization, compilers will put the symbols in a
> "common" section, which `man 1 nm` says is also unitialized data. I
> didn't think .bss sections occupied space in an object file or binary;
> the kernel's loader would set up the mappings at execution? Can you
> clarify?
Explicitly initialized global and static variables end up in the .data
section and need space in that section. That is not the case if the
initializer is left out and these variables end up in the .bss section.
>From https://en.wikipedia.org/wiki/.bss:
"In computer programming, the name .bss or bss is used by many compilers
and linkers for the portion of an object file or executable containing
statically-allocated variables that are not explicitly initialized to any
value. It is often referred to as the "bss section" or "bss segment".
Typically only the length of the bss section, but no data, is stored in
the object file."
This is why checkpatch warns if a global or static variable is initialized
explicitly to zero. From scripts/checkpatch.pl:
our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b};
# check for global initialisers.
if ($line =~
/^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) {
if (ERROR("GLOBAL_INITIALISERS",
"do not initialise globals to $1\n" .
$herecurr) && $fix) {
$fixed[$fixlinenr] =~
s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*$zero_initializer\s*;/$1;/;
}
}
# check for static initialisers.
if ($line =~ /^\+.*\bstatic\s.*=\s*($zero_initializer)\s*;/) {
if (ERROR("INITIALISED_STATIC",
"do not initialise statics to $1\n" .
$herecurr) && $fix) {
$fixed[$fixlinenr] =~
s/(\bstatic\s.*?)\s*=\s*$zero_initializer\s*;/$1;/;
}
}
Bart.
Re: [PATCH] libosd: Remove ignored __weak attribute
On Tue, Oct 2, 2018 at 10:57 AM Bart Van Assche wrote:
>
> On Tue, 2018-10-02 at 10:24 -0700, Nick Desaulniers wrote:
> > On Mon, Oct 1, 2018 at 6:16 PM Bart Van Assche wrote:
> > > Additionally, zero initializers should be left out to minimize
> > > the size of object files.
> >
> > Sorry, my understanding was that global variables either occupy the
> > .bss section or the .data section, depending on whether they were
> > zero-initialized vs initialized to non-zero, respectively (where
> > non-initialized are treated as zero initialized). Looks like without
> > the explicit zero initialization, compilers will put the symbols in a
> > "common" section, which `man 1 nm` says is also unitialized data. I
> > didn't think .bss sections occupied space in an object file or binary;
> > the kernel's loader would set up the mappings at execution? Can you
> > clarify?
>
> Explicitly initialized global and static variables end up in the .data
> section and need space in that section.
Unless the initial value is zero.
https://godbolt.org/z/curRoO
So you don't wind up with an increase in binary size simply by having
global variables initialized to zero, right? Instead the kernel knows
to create a zero'd out mapping for bss. You don't need a run of zeros
in the binary.
So I disagree when you said earlier "zero initializers should be left
out to minimize the size of object files." I assert they don't affect
the size of the binary.
If you had many global variables all initialized to zero, why would
you encode that many zeros in a binary, when you can just set a size
on the bss section and have the kernel create the appropriate sized
and zero'd mapping?
> That is not the case if the
> initializer is left out and these variables end up in the .bss section.
>From my above link, gcc will put globals without initializers into "common."
> From https://en.wikipedia.org/wiki/.bss:
>
> "In computer programming, the name .bss or bss is used by many compilers
> and linkers for the portion of an object file or executable containing
> statically-allocated variables that are not explicitly initialized to any
> value. It is often referred to as the "bss section" or "bss segment".
>
> Typically only the length of the bss section, but no data, is stored in
> the object file."
>
> This is why checkpatch warns if a global or static variable is initialized
> explicitly to zero. From scripts/checkpatch.pl:
>
> our $zero_initializer = qr{(?:(?:0[xX])?0+$Int_type?|NULL|false)\b};
>
> # check for global initialisers.
>
> if ($line =~
> /^\+$Type\s*$Ident(?:\s+$Modifier)*\s*=\s*($zero_initializer)\s*;/) {
> if (ERROR("GLOBAL_INITIALISERS",
> "do not initialise globals to $1\n" .
> $herecurr) && $fix) {
> $fixed[$fixlinenr] =~
> s/(^.$Type\s*$Ident(?:\s+$Modifier)*)\s*=\s*$zero_initializer\s*;/$1;/;
> }
> }
>
> # check for static initialisers.
>
> if ($line =~ /^\+.*\bstatic\s.*=\s*($zero_initializer)\s*;/) {
> if (ERROR("INITIALISED_STATIC",
> "do not initialise statics to $1\n" .
> $herecurr) && $fix) {
> $fixed[$fixlinenr] =~
> s/(\bstatic\s.*?)\s*=\s*$zero_initializer\s*;/$1;/;
> }
> }
>
> Bart.
--
Thanks,
~Nick Desaulniers
Re: [PATCH v2] scsi: bfa: Avoid implicit enum conversion in bfad_im_post_vendor_event
On Thu, Sep 27, 2018 at 4:57 PM Nathan Chancellor
wrote:
>
> Clang warns when one enumerated type is implicitly converted to another.
>
> drivers/scsi/bfa/bfa_fcs_lport.c:379:26: warning: implicit conversion
> from enumeration type 'enum bfa_lport_aen_event' to different
> enumeration type 'enum bfa_ioc_aen_event' [-Wenum-conversion]
> BFA_AEN_CAT_LPORT, event);
> ^
>
> The root cause of these warnings is the bfad_im_post_vendor_event
> function, which expects a value from enum bfa_ioc_aen_event but there
> are multiple instances of values from enums bfa_port_aen_event,
> bfa_audit_aen_event, and bfa_lport_aen_event being used in this
> function.
>
> Given that this doesn't appear to be a problem since cat helps with
> differentiating the events, just change evt's type to int so that no
> conversion needs to happen and Clang won't warn. Update aen_type's type
> in bfa_aen_entry_s as members that hold enumerated types should be int.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/147
> Signed-off-by: Nathan Chancellor
> ---
>
> v1 -> v2:
>
> * Update aen_type's type in bfa_aen_entry_s to match evt
Thanks for respinning with this addition.
Reviewed-by: Nick Desaulniers
>
> drivers/scsi/bfa/bfa_defs_svc.h | 2 +-
> drivers/scsi/bfa/bfad_im.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/scsi/bfa/bfa_defs_svc.h b/drivers/scsi/bfa/bfa_defs_svc.h
> index 3d0c96a5c873..c19c26e0e405 100644
> --- a/drivers/scsi/bfa/bfa_defs_svc.h
> +++ b/drivers/scsi/bfa/bfa_defs_svc.h
> @@ -1453,7 +1453,7 @@ union bfa_aen_data_u {
> struct bfa_aen_entry_s {
> struct list_headqe;
> enum bfa_aen_category aen_category;
> - u32 aen_type;
> + int aen_type;
> union bfa_aen_data_uaen_data;
> u64 aen_tv_sec;
> u64 aen_tv_usec;
> diff --git a/drivers/scsi/bfa/bfad_im.h b/drivers/scsi/bfa/bfad_im.h
> index e61ed8dad0b4..bd4ac187fd8e 100644
> --- a/drivers/scsi/bfa/bfad_im.h
> +++ b/drivers/scsi/bfa/bfad_im.h
> @@ -143,7 +143,7 @@ struct bfad_im_s {
> static inline void bfad_im_post_vendor_event(struct bfa_aen_entry_s *entry,
> struct bfad_s *drv, int cnt,
> enum bfa_aen_category cat,
> -enum bfa_ioc_aen_event evt)
> +int evt)
> {
> struct timespec64 ts;
>
> --
> 2.19.0
>
--
Thanks,
~Nick Desaulniers
Re: [PATCH] libosd: Remove ignored __weak attribute
On Tue, 2018-10-02 at 15:33 -0700, Nick Desaulniers wrote: > On Tue, Oct 2, 2018 at 10:57 AM Bart Van Assche wrote: > > Explicitly initialized global and static variables end up in the .data > > section and need space in that section. > > Unless the initial value is zero. > https://godbolt.org/z/curRoO > > So you don't wind up with an increase in binary size simply by having > global variables initialized to zero, right? Instead the kernel knows > to create a zero'd out mapping for bss. You don't need a run of zeros > in the binary. > > So I disagree when you said earlier "zero initializers should be left > out to minimize the size of object files." I assert they don't affect > the size of the binary. > > If you had many global variables all initialized to zero, why would > you encode that many zeros in a binary, when you can just set a size > on the bss section and have the kernel create the appropriate sized > and zero'd mapping? > > > That is not the case if the > > initializer is left out and these variables end up in the .bss section. > > From my above link, gcc will put globals without initializers into "common." No matter what particular compiler versions do with explicit initialization to zero, the preferred kernel coding style is to leave out such explicit initialization. Bart.
[PATCH 09/16] scsi: Replace spin_is_locked() with lockdep
lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked(). Signed-off-by: Lance Roy Cc: Karan Tilak Kumar Cc: Sesidhar Baddela Cc: "James E.J. Bottomley" Cc: "Martin K. Petersen" Cc: --- drivers/scsi/snic/snic_scsi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/snic/snic_scsi.c b/drivers/scsi/snic/snic_scsi.c index d9b2e46424aa..42e485139fc9 100644 --- a/drivers/scsi/snic/snic_scsi.c +++ b/drivers/scsi/snic/snic_scsi.c @@ -2001,7 +2001,7 @@ snic_dr_finish(struct snic *snic, struct scsi_cmnd *sc) } dr_failed: - SNIC_BUG_ON(!spin_is_locked(io_lock)); + lockdep_assert_held(io_lock); if (rqi) CMD_SP(sc) = NULL; spin_unlock_irqrestore(io_lock, flags); @@ -2604,7 +2604,7 @@ snic_internal_abort_io(struct snic *snic, struct scsi_cmnd *sc, int tmf) ret = SUCCESS; skip_internal_abts: - SNIC_BUG_ON(!spin_is_locked(io_lock)); + lockdep_assert_held(io_lock); spin_unlock_irqrestore(io_lock, flags); return ret; -- 2.19.0
Looking for some help understanding error handling
I'm working on LLDD for a SAS/SATA host adapter, and trying to understand how the system handles link loss and recovery. Say I have a device that gets recognized and attached as sd 12:0:4:0, at /dev/sdb. The drive goes offline temporarily, then comes back online. When it does, it comes back as sd 12:0:5:0, and maybe /dev/sdb, maybe /dev/sdc. I'm not sure how the Id gets assigned. Since this is the same drive, is there some way my driver can tell libsas and/or SCSI core that it's the same drive coming back? Or is there no way to control that? I looked into /dev/disk/by-id, but that also didn't quite do what I expected. If I open /dev/disk/by-id/some_identifier, that's a symlink to, say, /dev/sdb. /dev/sdb goes away, comes back as /dev/sdc, but my process doesn't know that, it still has /dev/disk/by-id/some_identifier opened and so it will never recover without closing and reopening the file. Thanks for any help or insight you can give. Chris Moore
[PATCH 0/3] sd: Rely on the driver core for asynchronous probing
Hello Martin, During the 2018 edition of LSF/MM there was a session about increasing SCSI disk probing concurrency. This patch series implements what has been proposed during that session, namely: - Make sure that the driver core is aware of asynchronous SCSI LUN probing. - Avoid unnecessary serialization between sd_probe() and sd_remove() because this could lead to a deadlock. Please consider this patch series for kernel v4.20. Thanks, Bart. Bart Van Assche (3): sd: Rely on the driver core for asynchronous probing sd: Inline sd_probe_part2() __device_release_driver(): Do not wait for asynchronous probing drivers/base/dd.c| 3 -- drivers/scsi/scsi.c | 14 - drivers/scsi/scsi_pm.c | 22 +--- drivers/scsi/scsi_priv.h | 3 -- drivers/scsi/sd.c| 110 --- 5 files changed, 46 insertions(+), 106 deletions(-) -- 2.19.0.605.g01d371f741-goog
[PATCH 3/3] __device_release_driver(): Do not wait for asynchronous probing
Since __device_release_driver() is called with the device lock held
and since the same device lock is obtained by the functions that
perform asynchronous probing (driver_attach_async() and
__device_attach_async_helper()), asynchronous probing is already
serialized against __device_release_driver(). Remove the
async_synchronize_full() call from __device_release_driver() to
avoid that a deadlock is triggered.
See also commit 765230b5f084 ("driver-core: add asynchronous probing
support for drivers").
Signed-off-by: Bart Van Assche
Cc: Dmitry Torokhov
Cc: Lee Duncan
Cc: Hannes Reinecke
Cc: Luis R. Rodriguez
Cc: Johannes Thumshirn
Cc: Christoph Hellwig
Cc: Greg Kroah-Hartman
---
drivers/base/dd.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index edfc9f0b1180..d6520de659bd 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -926,9 +926,6 @@ static void __device_release_driver(struct device *dev,
struct device *parent)
drv = dev->driver;
if (drv) {
- if (driver_allows_async_probing(drv))
- async_synchronize_full();
-
while (device_links_busy(dev)) {
device_unlock(dev);
if (parent)
--
2.19.0.605.g01d371f741-goog
[PATCH 1/3] sd: Rely on the driver core for asynchronous probing
As explained during the LSF/MM session about increasing SCSI disk
probing concurrency, the problems with the current probing approach
are as follows:
- The driver core is unaware of asynchronous SCSI LUN probing.
wait_for_device_probe() waits for all asynchronous probes except
asynchronous SCSI disk probes.
- There is unnecessary serialization between sd_probe() and sd_remove().
This can lead to a deadlock.
Hence this patch that modifies the sd driver such that it uses the
driver core framework for asynchronous probing. The async domains
and get_device()/put_device() pairs that became superfluous due to
this change are removed.
See also commit 3c31b52f96f7 ("scsi: async sd resume").
Signed-off-by: Bart Van Assche
Cc: Lee Duncan
Cc: Hannes Reinecke
Cc: Luis R. Rodriguez
Cc: Johannes Thumshirn
Cc: Christoph Hellwig
Cc: Greg Kroah-Hartman
Cc: Dan Williams
---
drivers/scsi/scsi.c | 14 --
drivers/scsi/scsi_pm.c | 22 ++
drivers/scsi/scsi_priv.h | 3 ---
drivers/scsi/sd.c| 13 +++--
4 files changed, 5 insertions(+), 47 deletions(-)
diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
index fc1356d101b0..1205369ad44f 100644
--- a/drivers/scsi/scsi.c
+++ b/drivers/scsi/scsi.c
@@ -85,19 +85,6 @@ unsigned int scsi_logging_level;
EXPORT_SYMBOL(scsi_logging_level);
#endif
-/* sd, scsi core and power management need to coordinate flushing async
actions */
-ASYNC_DOMAIN(scsi_sd_probe_domain);
-EXPORT_SYMBOL(scsi_sd_probe_domain);
-
-/*
- * Separate domain (from scsi_sd_probe_domain) to maximize the benefit of
- * asynchronous system resume operations. It is marked 'exclusive' to avoid
- * being included in the async_synchronize_full() that is invoked by
- * dpm_resume()
- */
-ASYNC_DOMAIN_EXCLUSIVE(scsi_sd_pm_domain);
-EXPORT_SYMBOL(scsi_sd_pm_domain);
-
/**
* scsi_put_command - Free a scsi command block
* @cmd: command block to free
@@ -839,7 +826,6 @@ static void __exit exit_scsi(void)
scsi_exit_devinfo();
scsi_exit_procfs();
scsi_exit_queue();
- async_unregister_domain(&scsi_sd_probe_domain);
}
subsys_initcall(init_scsi);
diff --git a/drivers/scsi/scsi_pm.c b/drivers/scsi/scsi_pm.c
index b44c1bb687a2..f229208bfef3 100644
--- a/drivers/scsi/scsi_pm.c
+++ b/drivers/scsi/scsi_pm.c
@@ -54,9 +54,6 @@ static int scsi_dev_type_suspend(struct device *dev,
const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
int err;
- /* flush pending in-flight resume operations, suspend is synchronous */
- async_synchronize_full_domain(&scsi_sd_pm_domain);
-
err = scsi_device_quiesce(to_scsi_device(dev));
if (err == 0) {
err = cb(dev, pm);
@@ -149,18 +146,7 @@ static int scsi_bus_resume_common(struct device *dev,
if (scsi_is_sdev_device(dev) && pm_runtime_suspended(dev))
blk_set_runtime_active(to_scsi_device(dev)->request_queue);
- if (fn) {
- async_schedule_domain(fn, dev, &scsi_sd_pm_domain);
-
- /*
-* If a user has disabled async probing a likely reason
-* is due to a storage enclosure that does not inject
-* staggered spin-ups. For safety, make resume
-* synchronous as well in that case.
-*/
- if (strncmp(scsi_scan_type, "async", 5) != 0)
- async_synchronize_full_domain(&scsi_sd_pm_domain);
- } else {
+ if (!fn) {
pm_runtime_disable(dev);
pm_runtime_set_active(dev);
pm_runtime_enable(dev);
@@ -170,11 +156,7 @@ static int scsi_bus_resume_common(struct device *dev,
static int scsi_bus_prepare(struct device *dev)
{
- if (scsi_is_sdev_device(dev)) {
- /* sd probing uses async_schedule. Wait until it finishes. */
- async_synchronize_full_domain(&scsi_sd_probe_domain);
-
- } else if (scsi_is_host_device(dev)) {
+ if (scsi_is_host_device(dev)) {
/* Wait until async scanning is finished */
scsi_complete_async_scans();
}
diff --git a/drivers/scsi/scsi_priv.h b/drivers/scsi/scsi_priv.h
index 99f1db5e467e..b9fa363b4bbb 100644
--- a/drivers/scsi/scsi_priv.h
+++ b/drivers/scsi/scsi_priv.h
@@ -175,9 +175,6 @@ static inline int scsi_autopm_get_host(struct Scsi_Host *h)
{ return 0; }
static inline void scsi_autopm_put_host(struct Scsi_Host *h) {}
#endif /* CONFIG_PM */
-extern struct async_domain scsi_sd_pm_domain;
-extern struct async_domain scsi_sd_probe_domain;
-
/* scsi_dh.c */
#ifdef CONFIG_SCSI_DH
void scsi_dh_add_device(struct scsi_device *sdev);
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 4a57ffecc7e6..48a133a3aab8 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -559,6 +559,7 @@ static struct scsi_driver sd_template = {
.name = "sd",
.owner = THI
[PATCH 2/3] sd: Inline sd_probe_part2()
This patch does not change any functionality.
Signed-off-by: Bart Van Assche
Cc: Lee Duncan
Cc: Hannes Reinecke
Cc: Luis R. Rodriguez
Cc: Johannes Thumshirn
Cc: Christoph Hellwig
Cc: Greg Kroah-Hartman
---
drivers/scsi/sd.c | 101 --
1 file changed, 43 insertions(+), 58 deletions(-)
diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
index 48a133a3aab8..93732f1fb551 100644
--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3232,63 +3232,6 @@ static int sd_format_disk_name(char *prefix, int index,
char *buf, int buflen)
return 0;
}
-static void sd_probe_part2(struct scsi_disk *sdkp)
-{
- struct scsi_device *sdp;
- struct gendisk *gd;
- u32 index;
- struct device *dev;
-
- sdp = sdkp->device;
- gd = sdkp->disk;
- index = sdkp->index;
- dev = &sdp->sdev_gendev;
-
- gd->major = sd_major((index & 0xf0) >> 4);
- gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
-
- gd->fops = &sd_fops;
- gd->private_data = &sdkp->driver;
- gd->queue = sdkp->device->request_queue;
-
- /* defaults, until the device tells us otherwise */
- sdp->sector_size = 512;
- sdkp->capacity = 0;
- sdkp->media_present = 1;
- sdkp->write_prot = 0;
- sdkp->cache_override = 0;
- sdkp->WCE = 0;
- sdkp->RCD = 0;
- sdkp->ATO = 0;
- sdkp->first_scan = 1;
- sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
-
- sd_revalidate_disk(gd);
-
- gd->flags = GENHD_FL_EXT_DEVT;
- if (sdp->removable) {
- gd->flags |= GENHD_FL_REMOVABLE;
- gd->events |= DISK_EVENT_MEDIA_CHANGE;
- }
-
- blk_pm_runtime_init(sdp->request_queue, dev);
- device_add_disk(dev, gd);
- if (sdkp->capacity)
- sd_dif_config_host(sdkp);
-
- sd_revalidate_disk(gd);
-
- if (sdkp->security) {
- sdkp->opal_dev = init_opal_dev(sdp, &sd_sec_submit);
- if (sdkp->opal_dev)
- sd_printk(KERN_NOTICE, sdkp, "supports TCG Opal\n");
- }
-
- sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
- sdp->removable ? "removable " : "");
- scsi_autopm_put_device(sdp);
-}
-
/**
* sd_probe - called during driver initialization and whenever a
* new scsi device is attached to the system. It is called once
@@ -3378,7 +3321,49 @@ static int sd_probe(struct device *dev)
get_device(dev);
dev_set_drvdata(dev, sdkp);
- sd_probe_part2(sdkp);
+ gd->major = sd_major((index & 0xf0) >> 4);
+ gd->first_minor = ((index & 0xf) << 4) | (index & 0xfff00);
+
+ gd->fops = &sd_fops;
+ gd->private_data = &sdkp->driver;
+ gd->queue = sdkp->device->request_queue;
+
+ /* defaults, until the device tells us otherwise */
+ sdp->sector_size = 512;
+ sdkp->capacity = 0;
+ sdkp->media_present = 1;
+ sdkp->write_prot = 0;
+ sdkp->cache_override = 0;
+ sdkp->WCE = 0;
+ sdkp->RCD = 0;
+ sdkp->ATO = 0;
+ sdkp->first_scan = 1;
+ sdkp->max_medium_access_timeouts = SD_MAX_MEDIUM_TIMEOUTS;
+
+ sd_revalidate_disk(gd);
+
+ gd->flags = GENHD_FL_EXT_DEVT;
+ if (sdp->removable) {
+ gd->flags |= GENHD_FL_REMOVABLE;
+ gd->events |= DISK_EVENT_MEDIA_CHANGE;
+ }
+
+ blk_pm_runtime_init(sdp->request_queue, dev);
+ device_add_disk(dev, gd);
+ if (sdkp->capacity)
+ sd_dif_config_host(sdkp);
+
+ sd_revalidate_disk(gd);
+
+ if (sdkp->security) {
+ sdkp->opal_dev = init_opal_dev(sdp, &sd_sec_submit);
+ if (sdkp->opal_dev)
+ sd_printk(KERN_NOTICE, sdkp, "supports TCG Opal\n");
+ }
+
+ sd_printk(KERN_NOTICE, sdkp, "Attached SCSI %sdisk\n",
+ sdp->removable ? "removable " : "");
+ scsi_autopm_put_device(sdp);
return 0;
--
2.19.0.605.g01d371f741-goog
[Bug 201313] New: pm80xx mpi_ssp_completion 1883:SAS Address of IO Failure Drive:
https://bugzilla.kernel.org/show_bug.cgi?id=201313 Bug ID: 201313 Summary: pm80xx mpi_ssp_completion 1883:SAS Address of IO Failure Drive: Product: IO/Storage Version: 2.5 Kernel Version: 4.18.11-041811-generic Hardware: Other OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: SCSI Assignee: linux-scsi@vger.kernel.org Reporter: masterc...@hotmail.com Regression: No Created attachment 278901 --> https://bugzilla.kernel.org/attachment.cgi?id=278901&action=edit kernel logs for last few days PM8003 PMC-Sierra Rev3 card with rev5 firmware connected to NetApp DS4246 Shelf's The system was running fine with ubuntu 18.04 with 4.15 kernelthen whent to put another shelf in circulation and everything broke either 4.18.11-041811-generic or something else is causing the issue, replaced controller card with backup and also tried backup shelf with the same issue disks all passed badblock check after 160 hr scan time then when trying to format the SEAGATE ST33000650NS SM drives they kept dropping out or Pc completely freezing up aio@aio:~$ sudo mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 -v -L SRD0NA1B0 -m 1 /dev/sdac1 mke2fs 1.44.1 (24-Mar-2018) fs_types for mke2fs.conf resolution: 'ext4' Filesystem label=SRD0NA1B0 OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) Stride=0 blocks, Stripe width=0 blocks 183148544 inodes, 732566016 blocks 7325660 blocks (1.00%) reserved for the super user First data block=0 Maximum filesystem blocks=2881486848 22357 block groups 32768 blocks per group, 32768 fragments per group 8192 inodes per group Filesystem UUID: 1f85920d-cd5a-4120-bb84-e290cb8c8808 Superblock backups stored on blocks: 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208, 4096000, 7962624, 11239424, 2048, 23887872, 71663616, 78675968, 10240, 214990848, 51200, 550731776, 644972544 Allocating group tables: done Writing inode tables: done Creating journal (262144 blocks): done Writing superblocks and filesystem accounting information: Warning, had trouble writing out superblocks. aio@aio:~$ sudo mkfs.ext4 -E lazy_itable_init=0,lazy_journal_init=0 -v -L SRD0NA1B0 -m 1 /dev/sdac1 [sudo] password for aio: mke2fs 1.44.1 (24-Mar-2018) fs_types for mke2fs.conf resolution: 'ext4' Filesystem label=SRD0NA1B0 OS type: Linux Block size=4096 (log=2) Fragment size=4096 (log=2) Stride=0 blocks, Stripe width=0 blocks 183148544 inodes, 732566016 blocks 7325660 blocks (1.00%) reserved for the super user First data block=0 Maximum filesystem blocks=2881486848 22357 block groups 32768 blocks per group, 32768 fragments per group 8192 inodes per group Filesystem UUID: 6b4fa378-38aa-4c02-ad46-4e2876ce1cb1 Superblock backups stored on blocks: 32768, 98304, 163840, 229376, 294912, 819200, 884736, 1605632, 2654208, 4096000, 7962624, 11239424, 2048, 23887872, 71663616, 78675968, 10240, 214990848, 51200, 550731776, 644972544 Allocating group tables: done Writing inode tables: done Creating journal (262144 blocks): done Writing superblocks and filesystem accounting information: done -- You are receiving this mail because: You are the assignee for the bug.
[Bug 201313] pm80xx mpi_ssp_completion 1883:SAS Address of IO Failure Drive:
https://bugzilla.kernel.org/show_bug.cgi?id=201313 --- Comment #1 from MasterCATZ (masterc...@hotmail.com) --- pm80xx mpi_ssp_completion 1874:sas IO status 0x24 [ 708.424035] pm80xx mpi_ssp_completion 1883:SAS Address of IO Failure Drive:500605ba00b9cca2 [ 708.424241] sd 1:0:8:0: Power-on or device reset occurred [ 708.712116] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.072865] sas: Enter sas_scsi_recover_host busy: 1 failed: 1 [ 709.073843] sd 1:0:8:0: Power-on or device reset occurred [ 709.074231] sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 1 tries: 1 [ 709.082924] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.083028] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.083030] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.083032] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.083148] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.083151] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.083259] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION [ 709.473399] sas: Enter sas_scsi_recover_host busy: 7 failed: 7 [ 709.478894] sd 1:0:8:0: Power-on or device reset occurred [ 709.479203] sas: --- Exit sas_scsi_recover_host: busy: 0 failed: 7 tries: 1 [ 709.482057] pm80xx :04:00.0: dev 500605ba00b9cca2 sent sense data, but stat(28) is not CHECK CONDITION -- You are receiving this mail because: You are the assignee for the bug.
