| Message ID | 20201210092459.81939-1-hare@suse.de (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Mike Snitzer |
| Headers | show |
| Series | dm: avoid filesystem lookup in dm_get_dev_t() | expand |
On Thu, 2020-12-10 at 10:24 +0100, Hannes Reinecke wrote: > dm_get_dev_t() is just used to convert an arbitrary 'path' string > into a dev_t. It doesn't presume that the device is present; that > check will be done later, as the only caller is dm_get_device(), > which does a dm_get_table_device() later on, which will properly > open the device. > So if the path string already _is_ in major:minor representation > we can convert it directly, avoiding a recursion into the filesystem > to lookup the block device. > This avoids a hang in multipath_message() when the filesystem is > inaccessible. > > Signed-off-by: Hannes Reinecke <hare@suse.de> Ack, although I believe the scsi/__GENKSYMS__ part doesn't belong here. Note that this is essentially a revert/fix of 644bda6f3460 ("dm table: fall back to getting device using name_to_dev_t()"). Added the author of that patch to CC. Martin -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, 2020-12-10 at 18:11 +0100, Martin Wilck wrote: > On Thu, 2020-12-10 at 10:24 +0100, Hannes Reinecke wrote: > > dm_get_dev_t() is just used to convert an arbitrary 'path' string > > into a dev_t. It doesn't presume that the device is present; that > > check will be done later, as the only caller is dm_get_device(), > > which does a dm_get_table_device() later on, which will properly > > open the device. > > So if the path string already _is_ in major:minor representation > > we can convert it directly, avoiding a recursion into the > > filesystem > > to lookup the block device. > > This avoids a hang in multipath_message() when the filesystem is > > inaccessible. > > > > Signed-off-by: Hannes Reinecke <hare@suse.de> > > Ack, although I believe the scsi/__GENKSYMS__ part doesn't belong > here. > > Note that this is essentially a revert/fix of 644bda6f3460 ("dm > table: > fall back to getting device using name_to_dev_t()"). Added the author > of that patch to CC. Mike, do you need anything more to apply this one? Do you want a cleaned-up resend? Cheers, Martin -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jan 21 2021 at 10:02am -0500, Martin Wilck <mwilck@suse.com> wrote: > On Thu, 2020-12-10 at 18:11 +0100, Martin Wilck wrote: > > On Thu, 2020-12-10 at 10:24 +0100, Hannes Reinecke wrote: > > > dm_get_dev_t() is just used to convert an arbitrary 'path' string > > > into a dev_t. It doesn't presume that the device is present; that > > > check will be done later, as the only caller is dm_get_device(), > > > which does a dm_get_table_device() later on, which will properly > > > open the device. > > > So if the path string already _is_ in major:minor representation > > > we can convert it directly, avoiding a recursion into the > > > filesystem > > > to lookup the block device. > > > This avoids a hang in multipath_message() when the filesystem is > > > inaccessible. > > > > > > Signed-off-by: Hannes Reinecke <hare@suse.de> > > > > Ack, although I believe the scsi/__GENKSYMS__ part doesn't belong > > here. > > > > Note that this is essentially a revert/fix of 644bda6f3460 ("dm > > table: > > fall back to getting device using name_to_dev_t()"). Added the author > > of that patch to CC. > > Mike, do you need anything more to apply this one? Do you want a > cleaned-up resend? It got hung up with Christoph correctly requesting more discussion, last linux-block/lkml mail on the associated thread I kicked off is here: https://lkml.org/lkml/2020/12/23/76 Basically if Hannes or yourself would like to review that thread and send a relevant v2 it'd really help move this forward. I'm bogged down with too many competing tasks. You guys may be able to act on this line of development/fixing quicker than I'll get to it. Thanks, Mike -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, Jan 21, 2021 at 11:09:33AM -0500, Mike Snitzer wrote: > > Mike, do you need anything more to apply this one? Do you want a > > cleaned-up resend? > > It got hung up with Christoph correctly requesting more discussion, last > linux-block/lkml mail on the associated thread I kicked off is here: > https://lkml.org/lkml/2020/12/23/76 > > Basically if Hannes or yourself would like to review that thread and > send a relevant v2 it'd really help move this forward. I'm bogged down > with too many competing tasks. You guys may be able to act on this line > of development/fixing quicker than I'll get to it. I'll get back to this ASAP, sorry. -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
On Thu, 2021-01-21 at 18:03 +0100, Christoph Hellwig wrote: > On Thu, Jan 21, 2021 at 11:09:33AM -0500, Mike Snitzer wrote: > > > Mike, do you need anything more to apply this one? Do you want a > > > cleaned-up resend? > > > > It got hung up with Christoph correctly requesting more discussion, > > last > > linux-block/lkml mail on the associated thread I kicked off is > > here: > > https://lkml.org/lkml/2020/12/23/76 > > > > Basically if Hannes or yourself would like to review that thread > > and > > send a relevant v2 it'd really help move this forward. I'm bogged > > down > > with too many competing tasks. You guys may be able to act on this > > line > > of development/fixing quicker than I'll get to it. > > I'll get back to this ASAP, sorry. AFAICS reverting 644bda6f3460, which is basically what Hannes did, is consensus. Christoph suggested also to revert the export of name_to_dev_t(), but that is used in a couple of other places too, unexporting it will require a bit more work. I'd like to move forward with the issue of blocking dm ioctls, so I'll submit a v2. Feel free to use or discard it as you please. Thanks, Martin -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c index 0142dc0e6798..d71808be006b 100644 --- a/drivers/md/dm-table.c +++ b/drivers/md/dm-table.c @@ -411,7 +411,13 @@ dev_t dm_get_dev_t(const char *path) { dev_t dev; struct block_device *bdev; + unsigned int maj, min; + if (sscanf(path, "%u:%u", &maj, &min) == 2) { + dev = MKDEV(maj, min); + if (maj == MAJOR(dev) && min == MINOR(dev)) + return dev; + } bdev = lookup_bdev(path); if (IS_ERR(bdev)) dev = name_to_dev_t(path); diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c index 6e01e90d83af..deee3e99aed0 100644 --- a/drivers/scsi/scsi_error.c +++ b/drivers/scsi/scsi_error.c @@ -133,16 +133,6 @@ scmd_eh_abort_handler(struct work_struct *work) struct scsi_device *sdev = scmd->device; int rtn; -#ifndef __GENKSYMS__ - if (scmd->eh_eflags & SCSI_EH_INTERNAL_TIMEOUT) { - SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd, - "internal timeout\n")); - set_host_byte(scmd, DID_TIME_OUT); - scmd->eh_eflags &= ~SCSI_EH_INTERNAL_TIMEOUT; - scsi_finish_command(scmd); - return; - } -#endif if (scsi_host_eh_past_deadline(sdev->host)) { SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd, @@ -934,6 +924,14 @@ static int scsi_try_bus_device_reset(struct scsi_cmnd *scmd) static int scsi_try_to_abort_cmd(struct scsi_host_template *hostt, struct scsi_cmnd *scmd) { +#ifndef __GENKSYMS__ + if (scmd->eh_eflags & SCSI_EH_INTERNAL_TIMEOUT) { + SCSI_LOG_ERROR_RECOVERY(3, scmd_printk(KERN_INFO, scmd, + "internal timeout\n")); + scmd->eh_eflags &= ~SCSI_EH_INTERNAL_TIMEOUT; + return SUCCESS; + } +#endif if (!hostt->eh_abort_handler) return FAILED;
dm_get_dev_t() is just used to convert an arbitrary 'path' string into a dev_t. It doesn't presume that the device is present; that check will be done later, as the only caller is dm_get_device(), which does a dm_get_table_device() later on, which will properly open the device. So if the path string already _is_ in major:minor representation we can convert it directly, avoiding a recursion into the filesystem to lookup the block device. This avoids a hang in multipath_message() when the filesystem is inaccessible. Signed-off-by: Hannes Reinecke <hare@suse.de> --- drivers/md/dm-table.c | 6 ++++++ drivers/scsi/scsi_error.c | 18 ++++++++---------- 2 files changed, 14 insertions(+), 10 deletions(-)