| Message ID | 20240906015847.229539-3-dlemoal@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Code cleanup and memory usage reduction | expand |
On Fri, Sep 06, 2024 at 10:58:40AM +0900, Damien Le Moal wrote: > A struct ata_device flags are always set iand cleared with the device s/iand/and/ I can see many cases function that currently set or clear dev->flags withouth holding ap->lock, done by functions that run in EH context, which will thus not hold ap->lock. Perhaps rephrase this to: struct ata_device flags should always be set and cleared with the device > port locked. Testing for a flag should thus also be done while holding > the device port lock. In accordance to this, modify > ata_scsi_handle_link_detach() to both test and clear the > ATA_DFLAG_DETACHED flag while holding the device port lock. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/ata/libata-scsi.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c > index a3ffce4b218d..1a135d44286c 100644 > --- a/drivers/ata/libata-scsi.c > +++ b/drivers/ata/libata-scsi.c > @@ -4616,10 +4616,12 @@ static void ata_scsi_handle_link_detach(struct ata_link *link) > ata_for_each_dev(dev, link, ALL) { > unsigned long flags; > > - if (!(dev->flags & ATA_DFLAG_DETACHED)) > + spin_lock_irqsave(ap->lock, flags); > + if (!(dev->flags & ATA_DFLAG_DETACHED)) { > + spin_unlock_irqrestore(ap->lock, flags); > continue; > + } > > - spin_lock_irqsave(ap->lock, flags); > dev->flags &= ~ATA_DFLAG_DETACHED; > spin_unlock_irqrestore(ap->lock, flags); > > -- > 2.46.0 > Reviewed-by: Niklas Cassel <cassel@kernel.org>
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index a3ffce4b218d..1a135d44286c 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4616,10 +4616,12 @@ static void ata_scsi_handle_link_detach(struct ata_link *link) ata_for_each_dev(dev, link, ALL) { unsigned long flags; - if (!(dev->flags & ATA_DFLAG_DETACHED)) + spin_lock_irqsave(ap->lock, flags); + if (!(dev->flags & ATA_DFLAG_DETACHED)) { + spin_unlock_irqrestore(ap->lock, flags); continue; + } - spin_lock_irqsave(ap->lock, flags); dev->flags &= ~ATA_DFLAG_DETACHED; spin_unlock_irqrestore(ap->lock, flags);
A struct ata_device flags are always set iand cleared with the device port locked. Testing for a flag should thus also be done while holding the device port lock. In accordance to this, modify ata_scsi_handle_link_detach() to both test and clear the ATA_DFLAG_DETACHED flag while holding the device port lock. Signed-off-by: Damien Le Moal <dlemoal@kernel.org> --- drivers/ata/libata-scsi.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)