[Bug 151661] Adaptec 3805 prints "AAC: Host adapter dead -1" every 10 seconds but works fine anyway
https://bugzilla.kernel.org/show_bug.cgi?id=151661 Harold Snel changed: What|Removed |Added CC||har...@snel.ws --- Comment #7 from Harold Snel --- I have the exact same issue on my server running with an Adaptec 3405. The root system is mounted on that controller on an SSD harddisk. Until I upgraded from 14.04 TLS to 16.04 TLS it was working fine. After I upgraded to an new kernel first (from 4.2.0-41 to 4.2.0-42) the issue with the 'dead adapter' started. To test if a full system upgrade to 16.04 TLS would help I upgrade the system to that release. But on that version the problem is still there and the system is not usable. Now I'm running on 16.04 TLS but with the older kernel (4.2.0-41) and thats working just fine. But I want to use the newest kernel because thats how it should be :-) Here is some controller info: === root@server:~# lspci -nnvs 03:0e.0 03:0e.0 RAID bus controller [0104]: Adaptec AAC-RAID [9005:0285] Subsystem: Adaptec 3405 [9005:02bb] Flags: bus master, stepping, 66MHz, medium devsel, latency 64, IRQ 18 Memory at fda0 (64-bit, non-prefetchable) [size=2M] Expansion ROM at fdcc [disabled] [size=256K] Capabilities: [c0] Power Management version 2 Capabilities: [d0] MSI: Enable- Count=1/2 Maskable- 64bit+ Capabilities: [e0] PCI-X non-bridge device Kernel driver in use: aacraid Kernel modules: aacraid === -- You are receiving this mail because: You are watching the assignee of the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] ses: use scsi_is_sas_rphy instead of is_sas_attached
Use scsi_is_sas_rphy() instead of is_sas_attached() to decide whether we should obtain the SAS address from a scsi device or not. This will prevent us from tripping on the BUG_ON() in sas_sdev_to_rdev() if the rphy isn't attached to the SAS transport class, like it is with hpsa's logical devices. Fixes: 3f8d6f2a0 ('ses: fix discovery of SATA devices in SAS enclosures') Cc: sta...@vger.kernel.org # v4.5+ Signed-off-by: Johannes Thumshirn --- drivers/scsi/ses.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 53ef1cb6..1d82053 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -587,7 +587,7 @@ static void ses_match_to_enclosure(struct enclosure_device *edev, ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0); - if (is_sas_attached(sdev)) + if (scsi_is_sas_rphy(&sdev->sdev_gendev)) efd.addr = sas_get_address(sdev); if (efd.addr) { -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] sas: remove is_sas_attached()
As there are no more users of is_sas_attached() left, remove it. Signed-off-by: Johannes Thumshirn --- drivers/scsi/scsi_transport_sas.c | 16 include/scsi/scsi_transport_sas.h | 9 - 2 files changed, 25 deletions(-) diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c index 3f0ff07..60b651b 100644 --- a/drivers/scsi/scsi_transport_sas.c +++ b/drivers/scsi/scsi_transport_sas.c @@ -341,22 +341,6 @@ static int do_sas_phy_delete(struct device *dev, void *data) } /** - * is_sas_attached - check if device is SAS attached - * @sdev: scsi device to check - * - * returns true if the device is SAS attached - */ -int is_sas_attached(struct scsi_device *sdev) -{ - struct Scsi_Host *shost = sdev->host; - - return shost->transportt->host_attrs.ac.class == - &sas_host_class.class; -} -EXPORT_SYMBOL(is_sas_attached); - - -/** * sas_remove_children - tear down a devices SAS data structures * @dev: device belonging to the sas object * diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h index 13c0b2b..140 100644 --- a/include/scsi/scsi_transport_sas.h +++ b/include/scsi/scsi_transport_sas.h @@ -10,15 +10,6 @@ struct scsi_transport_template; struct sas_rphy; struct request; -#if !IS_ENABLED(CONFIG_SCSI_SAS_ATTRS) -static inline int is_sas_attached(struct scsi_device *sdev) -{ - return 0; -} -#else -extern int is_sas_attached(struct scsi_device *sdev); -#endif - static inline int sas_protocol_ata(enum sas_protocol proto) { return ((proto & SAS_PROTOCOL_SATA) || -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dm-mpath: always return reservation conflict
Not sure how Hannes' original patch was overlooked but... One issue I see with the patch is it will return -EBADE regardless of whether 'queue_if_no_path' is set. That's fine (since path isn't being failed for this case any more). But why not just return error immediately? But taking a step back, shouldn't all paths be tried once before returning an error? Obviously that'd impose the use of a new 'conflict_seen' (or whatever) flag at the end of 'struct pgpath'. And then only return error if the flag is set. I threw together the following RFC patch to illustrate what I'm thinking, but thinking about this further it is tough to know all paths have seen the reservation conflict (my patch assumes if 'conflict_seen' is set then the conflict iterated through all paths.. but if paths aren't being failed there isn't a guarantee that the path selector didn't just hand us back the same path that just experienced the conflict). So this is throw-away for now (and I'll get Hannes' patch applied for 4.8-rc3, with the tweak of returning -EBADE immediately): diff --git a/drivers/md/dm-mpath.c b/drivers/md/dm-mpath.c index ac734e5..c3d92db 100644 --- a/drivers/md/dm-mpath.c +++ b/drivers/md/dm-mpath.c @@ -41,6 +41,7 @@ struct pgpath { struct delayed_work activate_path; bool is_active:1; /* Path status */ + bool conflict_seen:1; }; #define path_to_pgpath(__pgp) container_of((__pgp), struct pgpath, path) @@ -1569,6 +1570,33 @@ static int do_end_io(struct multipath *m, struct request *clone, if (noretry_error(error)) return error; + if (error == -EBADE) { + /* +* EBADE signals a reservation conflict. +* We shouldn't fail the path here as we can communicate with +* the target. We should failover to the next path, but in +* doing so we might be causing a ping-pong between paths. +* Avoid this by only returning the reservation conflict error +* if a conflict has been seen on all paths. +*/ + if (!mpio->pgpath || mpio->pgpath->conflict_seen) { + struct priority_group *pg; + struct pgpath *p; + + /* clear 'conflict_seen' for all pgpaths */ + list_for_each_entry(pg, &m->priority_groups, list) { + list_for_each_entry(p, &pg->pgpaths, list) { + p->conflict_seen = false; + } + } + return error; + } + else if (mpio->pgpath) { + mpio->pgpath->conflict_seen = true; + return r; + } + } + if (mpio->pgpath) fail_path(mpio->pgpath); @@ -1576,9 +1604,6 @@ static int do_end_io(struct multipath *m, struct request *clone, if (!test_bit(MPATHF_QUEUE_IF_NO_PATH, &m->flags)) { if (!must_push_back_rq(m)) r = -EIO; - } else { - if (error == -EBADE) - r = error; } } -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: dm-mpath: always return reservation conflict
On Mon, Aug 15 2016 at 9:08P -0400, Mike Snitzer wrote: > Not sure how Hannes' original patch was overlooked but... It wasn't overlooked. It was very much unresolved. The original thread unraveled to all sorts of PR edge case concerns (and doubt about whether anything relies on the current multipath handling of reservation conflicts). See patchwork thread below. Obviously you have found a problematic case which requires Hannes' patch. So there is definitely increased pressure to fix this. > One issue I see with the patch is it will return -EBADE regardless of > whether 'queue_if_no_path' is set. That's fine (since path isn't being > failed for this case any more). But why not just return error > immediately? > > But taking a step back, shouldn't all paths be tried once before > returning an error? Obviously that'd impose the use of a new > 'conflict_seen' (or whatever) flag at the end of 'struct pgpath'. And > then only return error if the flag is set. > > I threw together the following RFC patch to illustrate what I'm > thinking, but thinking about this further it is tough to know all paths > have seen the reservation conflict (my patch assumes if 'conflict_seen' > is set then the conflict iterated through all paths.. but if paths > aren't being failed there isn't a guarantee that the path selector > didn't just hand us back the same path that just experienced the > conflict). Seems we still need a more sophisticated approach. But I'm left wondering: if we didn't do it would anything notice? Sadly, the same big question from the original thread from a year ago: https://patchwork.kernel.org/patch/6797111/ > So this is throw-away for now (and I'll get Hannes' patch applied for > 4.8-rc3, with the tweak of returning -EBADE immediately): Unfortunately, I'm _not_ staging Hannes' patch until I have James Bottomley's Ack (given his original issues with the patch haven't been explained away AFAICT). Mike -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ses: use scsi_is_sas_rphy instead of is_sas_attached
Hi Johannes, [auto build test ERROR on scsi/for-next] [also build test ERROR on v4.8-rc2 next-20160815] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Johannes-Thumshirn/ses-use-scsi_is_sas_rphy-instead-of-is_sas_attached/20160815-165716 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: s390-zfcpdump_defconfig (attached as .config) compiler: s390x-linux-gnu-gcc (Debian 5.4.0-6) 5.4.0 20160609 reproduce: wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=s390 All errors (new ones prefixed by >>): drivers/built-in.o: In function `ses_match_to_enclosure': >> drivers/scsi/ses.c:590: undefined reference to `scsi_is_sas_rphy' >> drivers/scsi/ses.c:591: undefined reference to `sas_get_address' vim +590 drivers/scsi/ses.c 584 struct efd efd = { 585 .addr = 0, 586 }; 587 588 ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0); 589 > 590 if (scsi_is_sas_rphy(&sdev->sdev_gendev)) > 591 efd.addr = sas_get_address(sdev); 592 593 if (efd.addr) { 594 efd.dev = &sdev->sdev_gendev; --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH 1/2] ses: use scsi_is_sas_rphy instead of is_sas_attached
On Mon, 2016-08-15 at 22:11 +0800, kbuild test robot wrote: > Hi Johannes, > > [auto build test ERROR on scsi/for-next] > [also build test ERROR on v4.8-rc2 next-20160815] > [if your patch is applied to the wrong git tree, please drop us a > note to help improve the system] This is happening because scsi_is_sas_rphy wasn't designed to be used outside the SAS transport class, so it's failing when the ATTRs aren't defined. This is the way you fix it (needs to be a precursor patch to 1/2): James --- diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h index 13c0b2b..0e1c50e 100644 --- a/include/scsi/scsi_transport_sas.h +++ b/include/scsi/scsi_transport_sas.h @@ -15,8 +15,13 @@ static inline int is_sas_attached(struct scsi_device *sdev) { return 0; } +static inline int scsi_is_sas_rphy(const struct device *) +{ + return 0; +} #else extern int is_sas_attached(struct scsi_device *sdev); +extern int scsi_is_sas_rphy(const struct device *); #endif static inline int sas_protocol_ata(enum sas_protocol proto) @@ -202,7 +207,6 @@ extern int sas_rphy_add(struct sas_rphy *); extern void sas_rphy_remove(struct sas_rphy *); extern void sas_rphy_delete(struct sas_rphy *); extern void sas_rphy_unlink(struct sas_rphy *); -extern int scsi_is_sas_rphy(const struct device *); struct sas_port *sas_port_alloc(struct device *, int); struct sas_port *sas_port_alloc_num(struct device *); -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ses: use scsi_is_sas_rphy instead of is_sas_attached
On Mon, Aug 15, 2016 at 07:25:53AM -0700, James Bottomley wrote: > On Mon, 2016-08-15 at 22:11 +0800, kbuild test robot wrote: > > Hi Johannes, > > > > [auto build test ERROR on scsi/for-next] > > [also build test ERROR on v4.8-rc2 next-20160815] > > [if your patch is applied to the wrong git tree, please drop us a > > note to help improve the system] > > This is happening because scsi_is_sas_rphy wasn't designed to be used > outside the SAS transport class, so it's failing when the ATTRs aren't > defined. > > This is the way you fix it (needs to be a precursor patch to 1/2): > > James Thanks, but I'm wondering about the call to sas_get_address(). It was there before the patch as well and now it's an undefined reference. Am I missing something here? Sure it can be guarded in the #if block. > > --- > > diff --git a/include/scsi/scsi_transport_sas.h > b/include/scsi/scsi_transport_sas.h > index 13c0b2b..0e1c50e 100644 > --- a/include/scsi/scsi_transport_sas.h > +++ b/include/scsi/scsi_transport_sas.h > @@ -15,8 +15,13 @@ static inline int is_sas_attached(struct scsi_device *sdev) > { > return 0; > } > +static inline int scsi_is_sas_rphy(const struct device *) > +{ > + return 0; > +} > #else > extern int is_sas_attached(struct scsi_device *sdev); > +extern int scsi_is_sas_rphy(const struct device *); > #endif > > static inline int sas_protocol_ata(enum sas_protocol proto) > @@ -202,7 +207,6 @@ extern int sas_rphy_add(struct sas_rphy *); > extern void sas_rphy_remove(struct sas_rphy *); > extern void sas_rphy_delete(struct sas_rphy *); > extern void sas_rphy_unlink(struct sas_rphy *); > -extern int scsi_is_sas_rphy(const struct device *); > > struct sas_port *sas_port_alloc(struct device *, int); > struct sas_port *sas_port_alloc_num(struct device *); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ses: use scsi_is_sas_rphy instead of is_sas_attached
On Mon, 2016-08-15 at 16:40 +0200, Johannes Thumshirn wrote: > On Mon, Aug 15, 2016 at 07:25:53AM -0700, James Bottomley wrote: > > On Mon, 2016-08-15 at 22:11 +0800, kbuild test robot wrote: > > > Hi Johannes, > > > > > > [auto build test ERROR on scsi/for-next] > > > [also build test ERROR on v4.8-rc2 next-20160815] > > > [if your patch is applied to the wrong git tree, please drop us a > > > note to help improve the system] > > > > This is happening because scsi_is_sas_rphy wasn't designed to be > > used outside the SAS transport class, so it's failing when the > > ATTRs aren't defined. > > > > This is the way you fix it (needs to be a precursor patch to 1/2): > > > > James > > Thanks, but I'm wondering about the call to sas_get_address(). It was > there before the patch as well and now it's an undefined reference. > Am I missing something here? Sure it can be guarded in the #if block. Yes: gcc is kind enough to elide any code that looks like if (0) { refer to something } So the static inline ensures anything within the brace isn't used as a linkable reference. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] ses: use scsi_is_sas_rphy instead of is_sas_attached
On Mon, Aug 15, 2016 at 07:59:25AM -0700, James Bottomley wrote: > On Mon, 2016-08-15 at 16:40 +0200, Johannes Thumshirn wrote: > > On Mon, Aug 15, 2016 at 07:25:53AM -0700, James Bottomley wrote: > > > On Mon, 2016-08-15 at 22:11 +0800, kbuild test robot wrote: > > > > Hi Johannes, > > > > > > > > [auto build test ERROR on scsi/for-next] > > > > [also build test ERROR on v4.8-rc2 next-20160815] > > > > [if your patch is applied to the wrong git tree, please drop us a > > > > note to help improve the system] > > > > > > This is happening because scsi_is_sas_rphy wasn't designed to be > > > used outside the SAS transport class, so it's failing when the > > > ATTRs aren't defined. > > > > > > This is the way you fix it (needs to be a precursor patch to 1/2): > > > > > > James > > > > Thanks, but I'm wondering about the call to sas_get_address(). It was > > there before the patch as well and now it's an undefined reference. > > Am I missing something here? Sure it can be guarded in the #if block. > > Yes: gcc is kind enough to elide any code that looks like > > if (0) { > refer to something > } > > So the static inline ensures anything within the brace isn't used as a > linkable reference. Aaahhh, that explains a lot. Anyways, I've included get_sas_address() in the ifdef guard and compile tested the new series with and with out CONFIG_SCSI_SAS_ATTRS. Will send out shortly. -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/3] sas: provide stub implementations for scsi_is_sas_phy and sas_get_address
Provide stub implementations for scsi_is_sas_phy and sas_get_address for kernel configurations which do not have CONFIG_SCSI_SAS_ATTRS defined. Reported-by: kbuild test robot Suggested-by: James Bottomley Signed-off-by: Johannes Thumshirn --- include/scsi/scsi_transport_sas.h | 14 -- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h index 13c0b2b..0fc648e 100644 --- a/include/scsi/scsi_transport_sas.h +++ b/include/scsi/scsi_transport_sas.h @@ -15,8 +15,20 @@ static inline int is_sas_attached(struct scsi_device *sdev) { return 0; } + +static inline int scsi_is_sas_phy(const struct device *sdev) +{ + return 0; +} + +static inline u64 sas_get_address(struct scsi_device *sdev) +{ + return 0; +} #else extern int is_sas_attached(struct scsi_device *sdev); +extern int scsi_is_sas_phy(const struct device *); +u64 sas_get_address(struct scsi_device *); #endif static inline int sas_protocol_ata(enum sas_protocol proto) @@ -187,9 +199,7 @@ extern struct sas_phy *sas_phy_alloc(struct device *, int); extern void sas_phy_free(struct sas_phy *); extern int sas_phy_add(struct sas_phy *); extern void sas_phy_delete(struct sas_phy *); -extern int scsi_is_sas_phy(const struct device *); -u64 sas_get_address(struct scsi_device *); unsigned int sas_tlr_supported(struct scsi_device *); unsigned int sas_is_tlr_enabled(struct scsi_device *); void sas_disable_tlr(struct scsi_device *); -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 0/3] Fix panic when a SES device is attached to a hpsa logical volume.
The first patch provides stub implementations for scsi_is_sas_phy() and sas_get_address() for the case that CONFIG_SCSI_SAS_ATTRS is not defined. The second patch implements the actual fix in ses.c by changing the is_sas_attached() call to scsi_is_sas_rphy(). The third and last patch removes is_sas_attached() as it doesn't have any more consumers left. Johannes Thumshirn (3): sas: provide stub implementations for scsi_is_sas_phy and sas_get_address ses: use scsi_is_sas_rphy instead of is_sas_attached sas: remove is_sas_attached() drivers/scsi/scsi_transport_sas.c | 16 drivers/scsi/ses.c| 2 +- include/scsi/scsi_transport_sas.h | 12 3 files changed, 9 insertions(+), 21 deletions(-) -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] ses: use scsi_is_sas_rphy instead of is_sas_attached
Use scsi_is_sas_rphy() instead of is_sas_attached() to decide whether we should obtain the SAS address from a scsi device or not. This will prevent us from tripping on the BUG_ON() in sas_sdev_to_rdev() if the rphy isn't attached to the SAS transport class, like it is with hpsa's logical devices. Fixes: 3f8d6f2a0 ('ses: fix discovery of SATA devices in SAS enclosures') Cc: sta...@vger.kernel.org # v4.5+ Suggested-by: Hannes Reinecke Signed-off-by: Johannes Thumshirn --- drivers/scsi/ses.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c index 53ef1cb6..1d82053 100644 --- a/drivers/scsi/ses.c +++ b/drivers/scsi/ses.c @@ -587,7 +587,7 @@ static void ses_match_to_enclosure(struct enclosure_device *edev, ses_enclosure_data_process(edev, to_scsi_device(edev->edev.parent), 0); - if (is_sas_attached(sdev)) + if (scsi_is_sas_rphy(&sdev->sdev_gendev)) efd.addr = sas_get_address(sdev); if (efd.addr) { -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] sas: remove is_sas_attached()
As there are no more users of is_sas_attached() left, remove it. Signed-off-by: Johannes Thumshirn --- drivers/scsi/scsi_transport_sas.c | 16 include/scsi/scsi_transport_sas.h | 6 -- 2 files changed, 22 deletions(-) diff --git a/drivers/scsi/scsi_transport_sas.c b/drivers/scsi/scsi_transport_sas.c index 3f0ff07..60b651b 100644 --- a/drivers/scsi/scsi_transport_sas.c +++ b/drivers/scsi/scsi_transport_sas.c @@ -341,22 +341,6 @@ static int do_sas_phy_delete(struct device *dev, void *data) } /** - * is_sas_attached - check if device is SAS attached - * @sdev: scsi device to check - * - * returns true if the device is SAS attached - */ -int is_sas_attached(struct scsi_device *sdev) -{ - struct Scsi_Host *shost = sdev->host; - - return shost->transportt->host_attrs.ac.class == - &sas_host_class.class; -} -EXPORT_SYMBOL(is_sas_attached); - - -/** * sas_remove_children - tear down a devices SAS data structures * @dev: device belonging to the sas object * diff --git a/include/scsi/scsi_transport_sas.h b/include/scsi/scsi_transport_sas.h index 0fc648e..ee4e8c1 100644 --- a/include/scsi/scsi_transport_sas.h +++ b/include/scsi/scsi_transport_sas.h @@ -11,11 +11,6 @@ struct sas_rphy; struct request; #if !IS_ENABLED(CONFIG_SCSI_SAS_ATTRS) -static inline int is_sas_attached(struct scsi_device *sdev) -{ - return 0; -} - static inline int scsi_is_sas_phy(const struct device *sdev) { return 0; @@ -26,7 +21,6 @@ static inline u64 sas_get_address(struct scsi_device *sdev) return 0; } #else -extern int is_sas_attached(struct scsi_device *sdev); extern int scsi_is_sas_phy(const struct device *); u64 sas_get_address(struct scsi_device *); #endif -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] fcoe: provide translation table between Ethernet and FC port speeds
Provide a translation table between Ethernet and FC port speeds so odd speeds (from a Ethernet POV) like 8 Gbit are correctly mapped to sysfs and open-fcoe's fcoeadm. Before: Description: BCM57840 NetXtreme II 10/20-Gigabit Ethernet Revision: 11 Manufacturer: Broadcom Corporation Serial Number:6CC2173EA1D0 Driver: bnx2x 1.712.30-0 Number of Ports: 1 Symbolic Name: bnx2fc (QLogic BCM57840) v2.10.3 over eth2 OS Device Name:host1 Node Name: 0x20006cc2173ea1d1 Port Name: 0x10006cc2173ea1d1 FabricName:0x10c0dd0ce717 Speed: unknown Supported Speed: 1 Gbit, 10 Gbit MaxFrameSize: 2048 bytes FC-ID (Port ID): 0x660702 State: Online After: Description: BCM57840 NetXtreme II 10/20-Gigabit Ethernet Revision: 11 Manufacturer: Broadcom Corporation Serial Number:6CC2173EA1D0 Driver: bnx2x 1.712.30-0 Number of Ports: 1 Symbolic Name: bnx2fc (QLogic BCM57840) v2.10.3 over eth2 OS Device Name:host1 Node Name: 0x20006cc2173ea1d1 Port Name: 0x10006cc2173ea1d1 FabricName:0x10c0dd0ce717 Speed: 8 Gbit Supported Speed: 1 Gbit, 10 Gbit MaxFrameSize: 2048 bytes FC-ID (Port ID): 0x660701 State: Online Signed-off-by: Johannes Thumshirn --- drivers/scsi/fcoe/fcoe_transport.c | 48 -- 1 file changed, 31 insertions(+), 17 deletions(-) diff --git a/drivers/scsi/fcoe/fcoe_transport.c b/drivers/scsi/fcoe/fcoe_transport.c index 7028dd3..3e7f0e7 100644 --- a/drivers/scsi/fcoe/fcoe_transport.c +++ b/drivers/scsi/fcoe/fcoe_transport.c @@ -83,6 +83,36 @@ static struct notifier_block libfcoe_notifier = { .notifier_call = libfcoe_device_notification, }; +static const struct { + u32 fc_port_speed; + u32 eth_port_speed; +} fcoe_port_speed_mapping[] = { + { FC_PORTSPEED_1GBIT, SPEED_1000 }, + { FC_PORTSPEED_2GBIT, 2000 }, + { FC_PORTSPEED_4GBIT, 4000 }, + { FC_PORTSPEED_8GBIT, 8000 }, + { FC_PORTSPEED_10GBIT, SPEED_1 }, + { FC_PORTSPEED_16GBIT, 16000 }, + { FC_PORTSPEED_20GBIT, SPEED_2 }, + { FC_PORTSPEED_25GBIT, SPEED_25000 }, + { FC_PORTSPEED_32GBIT, 32000 }, + { FC_PORTSPEED_40GBIT, SPEED_4 }, + { FC_PORTSPEED_50GBIT, SPEED_5 }, + { FC_PORTSPEED_100GBIT, SPEED_10 }, +}; + +static inline u32 eth2fc_speed(u32 eth_port_speed) +{ + int i; + + for (i = 0; i <= ARRAY_SIZE(fcoe_port_speed_mapping); i++) { + if (fcoe_port_speed_mapping[i].eth_port_speed == eth_port_speed) + return fcoe_port_speed_mapping[i].fc_port_speed; + } + + return FC_PORTSPEED_UNKNOWN; +} + /** * fcoe_link_speed_update() - Update the supported and actual link speeds * @lport: The local port to update speeds for @@ -126,23 +156,7 @@ int fcoe_link_speed_update(struct fc_lport *lport) SUPPORTED_4baseLR4_Full)) lport->link_supported_speeds |= FC_PORTSPEED_40GBIT; - switch (ecmd.base.speed) { - case SPEED_1000: - lport->link_speed = FC_PORTSPEED_1GBIT; - break; - case SPEED_1: - lport->link_speed = FC_PORTSPEED_10GBIT; - break; - case SPEED_2: - lport->link_speed = FC_PORTSPEED_20GBIT; - break; - case SPEED_4: - lport->link_speed = FC_PORTSPEED_40GBIT; - break; - default: - lport->link_speed = FC_PORTSPEED_UNKNOWN; - break; - } + lport->link_speed = eth2fc_speed(ecmd.base.speed); return 0; } return -1; -- 1.8.5.6 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: blacklist all RDAC devices for BLIST_NO_ULD_ATTACH
On 8/12/16, 5:56 PM, "Xose Vazquez Perez" wrote: "Universal Xport" LUN is used for in-band storage array management. Cc: Sean Stewart Cc: Christophe Varoqui Cc: James E.J. Bottomley Cc: Martin K. Petersen Cc: SCSI ML Cc: device-mapper development Signed-off-by: Xose Vazquez Perez --- Acked-by: Sean Stewart
Re: [GIT PULL] [PATCH v4 00/26] Delete CURRENT_TIME and CURRENT_TIME_SEC macros
On Sat, Aug 13, 2016 at 03:48:12PM -0700, Deepa Dinamani wrote: > The series is aimed at getting rid of CURRENT_TIME and CURRENT_TIME_SEC > macros. > The macros are not y2038 safe. There is no plan to transition them into being > y2038 safe. > ktime_get_* api's can be used in their place. And, these are y2038 safe. Who are you execting to pull this huge patch series? Why not just introduce the new api call, wait for that to be merged, and then push the individual patches through the different subsystems? After half of those get ignored, then provide a single set of patches that can go through Andrew or my trees. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH V4 0/2] smartpqi: initial commit of Microsemi smartpqi driver
> -Original Message- > From: Martin K. Petersen [mailto:martin.peter...@oracle.com] > Sent: Friday, August 12, 2016 3:10 PM > To: Don Brace > Cc: Martin K. Petersen; j...@linux.vnet.ibm.com; Viswas G; Mahesh > Rajashekhara; h...@infradead.org; Scott Teel; Kevin Barnett; Justin Lindley; > Scott Benesh; elli...@hpe.com; linux-scsi@vger.kernel.org > Subject: Re: [PATCH V4 0/2] smartpqi: initial commit of Microsemi smartpqi > driver > > EXTERNAL EMAIL > > > > "Don" == Don Brace writes: > > Don> Great. So we are good to go? > > I applied the driver to 4.9/scsi-queue. You'll need to come up with a > migration approach for patch 2. > > -- > Martin K. Petersen Oracle Linux Engineering I am thinking that you mean users will need to have some kind of notification that newer kernels will require that the smartpqi driver be configured, especially if they are already booting from the aacraid driver. Can I submit a smartpqi/Kconfig patch to change the default value to 'y' and add you as a Suggested-by? We will of course need some documentation as you suggested. Thanks, Don Brace ESC - Smart Storage Microsemi Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/3] sas: provide stub implementations for scsi_is_sas_phy and sas_get_address
On Mon, 2016-08-15 at 17:12 +0200, Johannes Thumshirn wrote: > +static inline u64 sas_get_address(struct scsi_device *sdev) > +{ > + return 0; > +} To be honest, we don't want this. We do want the code to fail to compile if you ever use sas_get_address where you shouldn't because the code will be incorrect at that point. If we could return an error, fine, but we just return the address. James -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 153171] New: scsi host6: runtime PM trying to activate child device host6 but parent (2-2:1.0) is not active
https://bugzilla.kernel.org/show_bug.cgi?id=153171 Bug ID: 153171 Summary: scsi host6: runtime PM trying to activate child device host6 but parent (2-2:1.0) is not active Product: IO/Storage Version: 2.5 Kernel Version: 4.8.0-040800rc2-lowlatency Hardware: All OS: Linux Tree: Mainline Status: NEW Severity: normal Priority: P1 Component: SCSI Assignee: linux-scsi@vger.kernel.org Reporter: carav...@gmail.com Regression: No Created attachment 228931 --> https://bugzilla.kernel.org/attachment.cgi?id=228931&action=edit dmesg_4.8.0-040800rc2-lowlatency Mount USB Pendrive dmesg: [ 4630.693915] scsi host6: runtime PM trying to activate child device host6 but parent (2-2:1.0) is not active -- You are receiving this mail because: You are the assignee for the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[Bug 153171] scsi host6: runtime PM trying to activate child device host6 but parent (2-2:1.0) is not active
https://bugzilla.kernel.org/show_bug.cgi?id=153171 --- Comment #1 from Cristian Aravena Romero --- Created attachment 228941 --> https://bugzilla.kernel.org/attachment.cgi?id=228941&action=edit lspci -vvnn -- You are receiving this mail because: You are the assignee for the bug. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Bug 153171] New: scsi host6: runtime PM trying to activate child device host6 but parent (2-2:1.0) is not active
Am 15.08.2016 um 21:55 schrieb bugzilla-dae...@bugzilla.kernel.org: > https://bugzilla.kernel.org/show_bug.cgi?id=153171 > > Bug ID: 153171 >Summary: scsi host6: runtime PM trying to activate child device > host6 but parent (2-2:1.0) is not active >Product: IO/Storage >Version: 2.5 > Kernel Version: 4.8.0-040800rc2-lowlatency > Hardware: All > OS: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: SCSI > Assignee: linux-scsi@vger.kernel.org > Reporter: carav...@gmail.com > Regression: No > > Created attachment 228931 > --> https://bugzilla.kernel.org/attachment.cgi?id=228931&action=edit > dmesg_4.8.0-040800rc2-lowlatency > > Mount USB Pendrive > > dmesg: > [ 4630.693915] scsi host6: runtime PM trying to activate child device host6 > but > parent (2-2:1.0) is not active > This issue is fixed by patch scsi: "core: configure runtime pm before calling device_add in scsi_add_host_with_dma" I sent on Aug 3rd. Regards, Heiner -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Time to make dynamically allocated devt the default for scsi disks?
On 08/14/2016 11:23 AM, Dan Williams wrote: [ adding Bart back to the cc ] On Sun, Aug 14, 2016 at 11:08 AM, Dan Williams wrote: On Sun, Aug 14, 2016 at 10:20 AM, James Bottomley wrote: [..] I like it. I still think the bdi registration code should be in charge of taking the extra reference on the disk device's parent to isolate / make clear why the extra reference is being acquired, but I won't lose sleep if Jens takes your smaller change. Thanks James! Bart, do you have a test configuration already set up for this case. Can you give the 2 patches from James a try? https://patchwork.kernel.org/patch/9278201/ https://patchwork.kernel.org/patch/9279513/ Hello Dan, The "sysfs: cannot create duplicate filename" issue is something I ran into sporadically before I started using my patch that fixes this issue. I have not yet found a systematic way to trigger this behavior. Anyway, I will drop my patch and will start testing James' two patches. It will take a few days to test these patches thoroughly. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/2] Block layer support ZAC/ZBC commands
Shaun, > On Aug 14, 2016, at 09:09, Shaun Tancheff wrote: […] >>> >> No, surely not. >> But one of the _big_ advantages for the RB tree is blkdev_discard(). >> Without the RB tree any mkfs program will issue a 'discard' for every >> sector. We will be able to coalesce those into one discard per zone, but >> we still need to issue one for _every_ zone. > > How can you make coalesce work transparently in the > sd layer _without_ keeping some sort of a discard cache along > with the zone cache? > > Currently the block layer's blkdev_issue_discard() is breaking > large discard's into nice granular and aligned chunks but it is > not preventing small discards nor coalescing them. > > In the sd layer would there be way to persist or purge an > overly large discard cache? What about honoring > discard_zeroes_data? Once the discard is completed with > discard_zeroes_data you have to return zeroes whenever > a discarded sector is read. Isn't that a log more than just > tracking a write pointer? Couldn't a zone have dozens of holes? My understanding of the standards regarding discard is that it is not mandatory and that it is a hint to the drive. The drive can completely ignore it if it thinks that is a better choice. I may be wrong on this though. Need to check again. For reset write pointer, the mapping to discard requires that the calls to blkdev_issue_discard be zone aligned for anything to happen. Specify less than a zone and nothing will be done. This I think preserve the discard semantic. As for the “discard_zeroes_data” thing, I also think that is a drive feature not mandatory. Drives may have it or not, which is consistent with the ZBC/ZAC standards regarding reading after write pointer (nothing says that zeros have to be returned). In any case, discard of CMR zones will be a nop, so for SMR drives, discard_zeroes_data=0 may be a better choice. > >> Which is (as indicated) really slow, and easily takes several minutes. >> With the RB tree we can short-circuit discards to empty zones, and speed >> up processing time dramatically. >> Sure we could be moving the logic into mkfs and friends, but that would >> require us to change the programs and agree on a library (libzbc?) which >> should be handling that. > > F2FS's mkfs.f2fs is already reading the zone topology via SG_IO ... > so I'm not sure your argument is valid here. This initial SMR support patch is just that: a first try. Jaegeuk used SG_IO (in fact copy-paste of parts of libzbc) because the current ZBC patch-set has no ioctl API for zone information manipulation. We will fix this mkfs.f2fs once we agree on an ioctl interface. > > [..] > 3) Try to condense the blkzone data structure to save memory: I think that we can at the very least remove the zone length, and also may be the per zone spinlock too (a single spinlock and proper state flags can be used). >>> >>> I have a variant that is an array of descriptors that roughly mimics the >>> api from blk-zoned.c that I did a few months ago as an example. >>> I should be able to update that to the current kernel + patches. >>> >> Okay. If we restrict the in-kernel SMR drive handling to devices with >> identical zone sizes of course we can remove the zone length. >> And we can do away with the per-zone spinlock and use a global one instead. > > I don't think dropping the zone length is a reasonable thing to do. > > What I propose is an array of _descriptors_ it doesn't drop _any_ > of the zone information that you are holding in an RB tree, it is > just a condensed format that _mostly_ plugs into your existing > API. I do not agree. The Seagate drive already has one zone (the last one) that is not the same length as the other zones. Sure, since it is the last one, we can had “if (last zone)” all over the place and make it work. But that is really ugly. Keeping the length field makes the code generic and following the standard, which has no restriction on the zone sizes. We could do some memory optimisation using different types of blk_zone sturcts, the types mapping to the SAME value: drives with constant zone size can use a blk_zone type without the length field, others use a different type that include the field. Accessor functions can hide the different types in the zone manipulation code. Best regards. -- Damien Le Moal, Ph.D. Sr. Manager, System Software Group, HGST Research, HGST, a Western Digital brand damien.lem...@hgst.com (+81) 0466-98-3593 (ext. 513593) 1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan www.hgst.com Western Digital Corporation (and its subsidiaries) E-mail Confidentiality Notice & Disclaimer: This e-mail and any files transmitted with it may contain confidential or legally privileged information of WDC and/or its affiliates, and are intended solely for the use of the individual or entity to which they are addressed. If you are not the intended recipient, any disclosure, copying, distribution or any action taken or omitte
Re: [RFC] sd: dynamically adjust SD_MAX_WS16_BLOCKS as per the actual logical block size
> "Tom" == Tom Yan writes: Tom, >> 0x7f, the maximum number of block layer sectors that can be >> expressed in a single bio. Tom> Hmm, so when we queue any of the limits, we convert a certain Tom> maximum number of physical sectors (which we has already been Tom> doing) logical sectors Tom> to its corresponding maximum number of block layer sectors, and Tom> then make sure that number does not exceed 0x7f, right? Yep. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/2] sd: check BLK_DEF_MAX_SECTORS against max_dev_sectors
> "Tom" == Tom Yan writes: Tom, >> It would be pretty unusual for a device that is smart enough to >> report a transfer length limit to be constrained to 1 MB and change. Tom> Well, it is done pretty much for libata's SATL. But why? >> rw_max = min(BLK_DEF_MAX_SECTORS, q->limits.max_dev_sectors); Tom> That won't really work. min_t() would, though the line is gonna be Tom> a bit long; not sure if I can/should simply cast the type (unsigned Tom> int) to BLK_DEF_MAX_SECTORS. And which braces are you referring to? I'd rather have a split line than double lines with braces. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] libata-scsi: make sure Maximum Write Same Length is not too large
> "Tom" == Tom Yan writes: Tom, Tom> The thing is, as of ACS-4, blocks that carry DSM/TRIM LBA Range Tom> Entries are always 512-byte. Lovely. And SAT conveniently ignores this entirely. Tom> Honestly, I have no idea how that would work on a 4Kn SSD, if it is Tom> / will ever be a thing. Highly unlikely. But I guess you would have to report 8 512-byte ranges in the 4Kn case. And then rely on the patch we talked about to clamp the number of sectors based on the block layer limit. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 2/3] ses: use scsi_is_sas_rphy instead of is_sas_attached
Hi Johannes, [auto build test ERROR on scsi/for-next] [also build test ERROR on v4.8-rc2 next-20160815] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Johannes-Thumshirn/Fix-panic-when-a-SES-device-is-attached-to-a-hpsa-logical-volume/20160815-231901 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: i386-randconfig-h0-08161012 (attached as .config) compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> ERROR: "scsi_is_sas_rphy" [drivers/scsi/ses.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: Binary data
Re: [PATCH] scsi: introduce a quirk for false cache reporting
> "Oliver" == Oliver Neukum writes: Oliver, Oliver> wce_default_on controls the default if the device provides no Oliver> indication. The problem here is that the indication the device Oliver> provides must be overridden, as they are false. Can't you just blacklist the mode select on the device in question? Something like: if (us->fflags & US_FL_ALWAYS_SYNC) { sdev->skip_ms_page_3f = 1; sdev->skip_ms_page_8 = 1; sdev->wce_default_on = 1; } ? -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] scsi: fix upper bounds check of sense key in scsi_sense_key_string()
> "Tyrel" == Tyrel Datwyler writes: Tyrel> Commit 655ee63cf371 added a "Completed" sense string with key 0xF Tyrel> to snstext[], but failed to updated the upper bounds check of the Tyrel> sense key in scsi_sense_key_string(). Applied to 4.8/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: blacklist all RDAC devices for BLIST_NO_ULD_ATTACH
> "Xose" == Xose Vazquez Perez writes: Xose> "Universal Xport" LUN is used for in-band storage array Xose> management. Applied to 4.8/scsi-fixes. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/2] Block layer support ZAC/ZBC commands
On Mon, Aug 15, 2016 at 11:00 PM, Damien Le Moal wrote: > > Shaun, > >> On Aug 14, 2016, at 09:09, Shaun Tancheff wrote: > […] >>> No, surely not. >>> But one of the _big_ advantages for the RB tree is blkdev_discard(). >>> Without the RB tree any mkfs program will issue a 'discard' for every >>> sector. We will be able to coalesce those into one discard per zone, but >>> we still need to issue one for _every_ zone. >> >> How can you make coalesce work transparently in the >> sd layer _without_ keeping some sort of a discard cache along >> with the zone cache? >> >> Currently the block layer's blkdev_issue_discard() is breaking >> large discard's into nice granular and aligned chunks but it is >> not preventing small discards nor coalescing them. >> >> In the sd layer would there be way to persist or purge an >> overly large discard cache? What about honoring >> discard_zeroes_data? Once the discard is completed with >> discard_zeroes_data you have to return zeroes whenever >> a discarded sector is read. Isn't that a log more than just >> tracking a write pointer? Couldn't a zone have dozens of holes? > > My understanding of the standards regarding discard is that it is not > mandatory and that it is a hint to the drive. The drive can completely > ignore it if it thinks that is a better choice. I may be wrong on this > though. Need to check again. But you are currently setting discard_zeroes_data=1 in your current patches. I believe that setting discard_zeroes_data=1 effectively promotes discards to being mandatory. I have a follow on patch to my SCT Write Same series that handles the CMR zone case in the sd_zbc_setup_discard() handler. > For reset write pointer, the mapping to discard requires that the calls > to blkdev_issue_discard be zone aligned for anything to happen. Specify > less than a zone and nothing will be done. This I think preserve the > discard semantic. Oh. If that is the intent then there is just a bug in the handler. I have pointed out where I believe it to be in my response to the zone cache patch being posted. > As for the “discard_zeroes_data” thing, I also think that is a drive > feature not mandatory. Drives may have it or not, which is consistent > with the ZBC/ZAC standards regarding reading after write pointer (nothing > says that zeros have to be returned). In any case, discard of CMR zones > will be a nop, so for SMR drives, discard_zeroes_data=0 may be a better > choice. However I am still curious about discard's being coalesced. >>> Which is (as indicated) really slow, and easily takes several minutes. >>> With the RB tree we can short-circuit discards to empty zones, and speed >>> up processing time dramatically. >>> Sure we could be moving the logic into mkfs and friends, but that would >>> require us to change the programs and agree on a library (libzbc?) which >>> should be handling that. >> >> F2FS's mkfs.f2fs is already reading the zone topology via SG_IO ... >> so I'm not sure your argument is valid here. > > This initial SMR support patch is just that: a first try. Jaegeuk > used SG_IO (in fact copy-paste of parts of libzbc) because the current > ZBC patch-set has no ioctl API for zone information manipulation. We > will fix this mkfs.f2fs once we agree on an ioctl interface. Which again is my point. If mkfs.f2fs wants to speed up it's discard pass in mkfs.f2fs by _not_ sending unneccessary Reset WP for zones that are already empty it has all the information it needs to do so. Here it seems to me that the zone cache is _at_best_ doing double work. At works the zone cache could be doing the wrong thing _if_ the zone cache got out of sync. It is certainly possible (however unlikely) that someone was doing some raw sg activity that is not seed by the sd path. All I am trying to do is have a discussion about the reasons for and against have a zone cache. Where it works and where it breaks this should be entirely technical but I understand that we have all spent a lot of time _not_ discussing this for various non-technical reasons. So far the only reason I've been able to ascertain is that Host Manged drives really don't like being stuck with the URSWRZ and would like to have a software hack to return MUD rather than ship drives with some weird out-of-the box config where the last zone is marked as FINISH'd thereby returning MUD on reads as per spec. I understand that it would be strange state to see of first boot and likely people would just do a ResetWP and have weird boot errors, which would probably just make matters worse. I just would rather the work around be a bit cleaner and/or use less memory. I would also like a path available that does not require SD_ZBC or BLK_ZONED for Host Aware drives to work, hence this set of patches and me begging for a single bit in struct bio. >> >> [..] >> > 3) Try to condense the blkzone data structure to save memory: > I think that we can at the very least remove the zone length, and also > may be t
Re: [PATCH v6 0/2] Block layer support ZAC/ZBC commands
On Tue, Aug 9, 2016 at 11:38 PM, Damien Le Moal wrote: > Shaun, > > On 8/10/16 12:58, Shaun Tancheff wrote: >> >> On Tue, Aug 9, 2016 at 3:09 AM, Damien Le Moal >> wrote: On Aug 9, 2016, at 15:47, Hannes Reinecke wrote: >> >> >> [trim] >> > Since disk type == 0 for everything that isn't HM so I would prefer the > sysfs 'zoned' file just report if the drive is HA or HM. > Okay. So let's put in the 'zoned' attribute the device type: 'host-managed', 'host-aware', or 'device managed'. >>> >>> >>> I hacked your patches and simply put a "0" or "1" in the sysfs zoned >>> file. >>> Any drive that has ZBC/ZAC command support gets a "1", "0" for everything >>> else. This means that drive managed models are not exposed as zoned block >>> devices. For HM vs HA differentiation, an application can look at the >>> device type file since it is already present. >>> >>> We could indeed set the "zoned" file to the device type, but HM drives >>> and >>> regular drives will both have "0" in it, so no differentiation possible. >>> The other choice could be the "zoned" bits defined by ZBC, but these >>> do not define a value for host managed drives, and the drive managed >>> value >>> being not "0" could be confusing too. So I settled for a simple 0/1 >>> boolean. >> >> >> This seems good to me. > > > Another option I forgot is for the "zoned" file to indicate the total number > of zones of the device, and 0 for a non zoned regular block device. That > would work as well. Clearly either is sufficient. > [...] >>> >>> Done: I hacked Shaun ioctl code and added finish zone too. The >>> difference with Shaun initial code is that the ioctl are propagated down >>> to >>> the driver (__blkdev_driver_ioctl -> sd_ioctl) so that there is no need >>> for >>> BIO request definition for the zone operations. So a lot less code added. >> >> >> The purpose of the BIO flags is not to enable the ioctls so much as >> the other way round. Creating BIO op's is to enable issuing ZBC >> commands from device mapper targets and file systems without some >> heinous ioctl hacks. >> Making the resulting block layer interfaces available via ioctls is just a >> reasonable way to exercise the code ... or that was my intent. > > > Yes, I understood your code. However, since (or if) we keep the zone > information in the RB-tree cache, there is no need for the report zone > operation BIO interface. Same for reset write pointer by keeping the mapping > to discard. blk_lookup_zone can be used in kernel as a report zone BIO > replacement and works as well for the report zone ioctl implementation. For > reset, there is blkdev_issue_discrad in kernel, and the reset zone ioctl > becomes equivalent to BLKDISCARD ioctl. These are simple. Open, close and > finish zone remains. For these, adding the BIO interface seemed an overkill. > Hence my choice of propagating the ioctl to the driver. > This is debatable of course, and adding an in-kernel interface is not hard: > we can implement blk_open_zone, blk_close_zone and blk_finish_zone using > __blkdev_driver_ioctl. That looks clean to me. Uh. I would call that "heinous" ioctl hacks myself. Kernel -> User API -> Kernel is not really a good designed IMO. > Overall, my concern with the BIO based interface for the ZBC commands is > that it adds one flag for each command, which is not really the philosophy > of the interface and potentially opens the door for more such > implementations in the future with new standards and new commands coming up. > Clearly that is not a sustainable path. So I think that a more specific > interface for these zone operations is a better choice. That is consistent > with what happens with the tons of ATA and SCSI commands not actually doing > data I/Os (mode sense, log pages, SMART, etc). All these do not use BIOs and > are processed as request REQ_TYPE_BLOCK_PC. Part of the reason for following on Mike Christie's bio op/flags cleanup was to make these op's. The advantage of being added as ops is that there is only 1 extra bit need (not 4 or 5 bits for flags). The other reason for being promoted into the block layer as commands is because it seems to me to make sense that these abstractions could be allowed to be passed through a DM layer and be handled by a files system. >>> The ioctls do not mimic exactly the ZBC standard. For instance, there is >>> no >>> reporting options for report zones, nor is the "all" bit supported for >>> open, >>> close or finish zone commands. But the information provided on zones is >>> complete >>> and maps to the standard definitions. >> >> >> For the reporting options I have planned to reuse the stream_id in >> struct bio when that is formalized. There are certainly other places in >> struct bio to stuff a few extra bits ... Sorry I was confused here. I was under the impression you were talking about one of my patches when you seem to have been talking about your hacking thereof. > We could add reporting options to blk_loo
RE: [PATCH] be2iscsi: Use a more current logging style
Thanks Joe for taking this up. It has been pending for long time from our side. > -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Joe Perches > Sent: Monday, August 15, 2016 12:26 AM > To: Christophe JAILLET; Jayamohan Kallickal; Ketan Mukadam > Cc: Bart Van Assche; James E.J. Bottomley; Martin K. Petersen; linux- > s...@vger.kernel.org; linux-ker...@vger.kernel.org > Subject: [PATCH] be2iscsi: Use a more current logging style > > shost_printk macros are converted to beiscsi_ for a > more current logging style. > > Consolidate the masked tests into a beiscsi_printk macro and > use that to call shost_printk. > > Convert the __beiscsi_log macros to use shost_print directly. > > Miscellanea: > > o Remove the file prefix identifier and use kbasename(__FILE__) > and stringify(__LINE__) to reduce code size in beiscsi_printk > o Realign arguments > > Signed-off-by: Joe Perches > --- > drivers/scsi/be2iscsi/be_cmds.c | 142 > drivers/scsi/be2iscsi/be_iscsi.c | 223 ++-- > drivers/scsi/be2iscsi/be_main.c | 733 +++ > drivers/scsi/be2iscsi/be_main.h | 20 +- > drivers/scsi/be2iscsi/be_mgmt.c | 270 +++--- > 5 files changed, 668 insertions(+), 720 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_cmds.c b/drivers/scsi/be2iscsi/be_cmds.c > index a51bc0e..574d2f4 100644 > --- a/drivers/scsi/be2iscsi/be_cmds.c > +++ b/drivers/scsi/be2iscsi/be_cmds.c > @@ -95,8 +95,8 @@ int be_chk_reset_complete(struct beiscsi_hba *phba) > } > > if ((status & 0x8000) || (!num_loop)) { > - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT, > - "BC", "Failed in be_chk_reset_complete status = > 0x%x\n", > + beiscsi_err(phba, BEISCSI_LOG_INIT, > + "Failed in be_chk_reset_complete status = 0x%x\n", > status); > return -EIO; > } > @@ -135,9 +135,9 @@ struct be_mcc_wrb *alloc_mcc_wrb(struct beiscsi_hba > *phba, > > spin_lock_bh(&phba->ctrl.mcc_lock); > if (mccq->used == mccq->len) { > - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT | > + beiscsi_err(phba, BEISCSI_LOG_INIT | > BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC", "MCC queue full: WRB used %u tag avail %u\n", > + "MCC queue full: WRB used %u tag avail %u\n", > mccq->used, phba->ctrl.mcc_tag_available); > goto alloc_failed; > } > @@ -147,9 +147,9 @@ struct be_mcc_wrb *alloc_mcc_wrb(struct beiscsi_hba > *phba, > > tag = phba->ctrl.mcc_tag[phba->ctrl.mcc_alloc_index]; > if (!tag) { > - beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT | > + beiscsi_err(phba, BEISCSI_LOG_INIT | > BEISCSI_LOG_CONFIG | BEISCSI_LOG_MBOX, > - "BC", "MCC tag 0 allocated: tag avail %u alloc index > %u\n", > + "MCC tag 0 allocated: tag avail %u alloc index %u\n", > phba->ctrl.mcc_tag_available, > phba->ctrl.mcc_alloc_index); > goto alloc_failed; > @@ -263,10 +263,10 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba > *phba, > set_bit(MCC_TAG_STATE_TIMEOUT, > &phba->ctrl.ptag_state[tag].tag_state); > > - beiscsi_log(phba, KERN_ERR, > + beiscsi_err(phba, > BEISCSI_LOG_INIT | BEISCSI_LOG_EH | > BEISCSI_LOG_CONFIG, > - "BC", "MBX Cmd Completion timed out\n"); > + "MBX Cmd Completion timed out\n"); > return -EBUSY; > } > > @@ -289,22 +289,22 @@ int beiscsi_mccq_compl_wait(struct beiscsi_hba > *phba, > } > > if (status || addl_status) { > - beiscsi_log(phba, KERN_WARNING, > - BEISCSI_LOG_INIT | BEISCSI_LOG_EH | > - BEISCSI_LOG_CONFIG, > - "BC", "MBX Cmd Failed for Subsys : %d Opcode : %d > with Status : %d and Extd_Status : %d\n", > - mbx_hdr->subsystem, > - mbx_hdr->opcode, > - status, addl_status); > + beiscsi_warn(phba, > + BEISCSI_LOG_INIT | BEISCSI_LOG_EH | > + BEISCSI_LOG_CONFIG, > + "MBX Cmd Failed for Subsys : %d Opcode : %d with > Status : %d and Extd_Status : %d\n", > + mbx_hdr->subsystem, > + mbx_hdr->opcode, > + status, addl_status); > rc = -EIO; > if (status == MCC_STATUS_INSUFFICIENT_BUFFER) { > mbx_resp_hdr = (struct be_cmd_resp_hdr *) mbx_hdr; > -
RE: [PATCH 1/2] be2iscsi: Fix error return code
> -Original Message- > From: linux-scsi-ow...@vger.kernel.org [mailto:linux-scsi- > ow...@vger.kernel.org] On Behalf Of Christophe JAILLET > Sent: Friday, August 12, 2016 3:33 PM > To: jayamohan.kallic...@avagotech.com; j...@linux.vnet.ibm.com; > ketan.muka...@avagotech.com; sony.j...@avagotech.com; > martin.peter...@oracle.com > Cc: linux-scsi@vger.kernel.org; linux-ker...@vger.kernel.org; kernel- > janit...@vger.kernel.org; Christophe JAILLET > Subject: [PATCH 1/2] be2iscsi: Fix error return code > > We know that 'ret' is not an error code because it has been tested a few > lines > above. > So, if one of these function fails, 0 will be returned instead of an error > code. > Return -ENOMEM instead. > > Signed-off-by: Christophe JAILLET > --- > drivers/scsi/be2iscsi/be_main.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/be2iscsi/be_main.c > b/drivers/scsi/be2iscsi/be_main.c > index f05e7737107d..89ae6390b697 100644 > --- a/drivers/scsi/be2iscsi/be_main.c > +++ b/drivers/scsi/be2iscsi/be_main.c > @@ -3286,8 +3286,10 @@ static int beiscsi_create_eqs(struct beiscsi_hba > *phba, > eq_vaddress = pci_alloc_consistent(phba->pcidev, >num_eq_pages * > PAGE_SIZE, >&paddr); > - if (!eq_vaddress) > + if (!eq_vaddress) { > + ret = -ENOMEM; > goto create_eq_error; > + } > > mem->va = eq_vaddress; > ret = be_fill_queue(eq, phba->params.num_eq_entries, @@ - > 3349,8 +3351,11 @@ static int beiscsi_create_cqs(struct beiscsi_hba *phba, > cq_vaddress = pci_alloc_consistent(phba->pcidev, >num_cq_pages * > PAGE_SIZE, >&paddr); > - if (!cq_vaddress) > + if (!cq_vaddress) { > + ret = -ENOMEM; > goto create_cq_error; > + } > + > ret = be_fill_queue(cq, phba->params.num_cq_entries, > sizeof(struct sol_cqe), cq_vaddress); > if (ret) { > @@ -5635,6 +5640,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev, > if (!phba) { > dev_err(&pcidev->dev, > "beiscsi_dev_probe - Failed in beiscsi_hba_alloc\n"); > + ret = -ENOMEM; > goto disable_pci; > } > > @@ -5754,6 +5760,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev, > beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT, > "BM_%d : beiscsi_dev_probe-" > "Failed to allocate work queue\n"); > + ret = -ENOMEM; > goto free_twq; > } > [JB] This still has to be fixed, right? -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html