All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@gmail.com>
To: Martin Wilck <mwilck@suse.com>
Cc: Christoph Hellwig <hch@lst.de>,
	Alasdair G Kergon <agk@redhat.com>,
	Bart Van Assche <Bart.VanAssche@sandisk.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, dm-devel@redhat.com,
	Hannes Reinecke <hare@suse.de>, Daniel Wagner <dwagner@suse.de>,
	linux-block@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
	Benjamin Marzinski <bmarzins@redhat.com>,
	nkoenig@redhat.com, emilne@redhat.com
Subject: Re: [PATCH v4 1/3] scsi: scsi_ioctl: export __scsi_result_to_blk_status()
Date: Wed, 30 Jun 2021 12:01:45 -0400	[thread overview]
Message-ID: <YNyVafnX09cOIZPe@redhat.com> (raw)
In-Reply-To: <1aa1f875e7a85f9a331e88e4f8482588176bdb3a.camel@suse.com>

On Wed, Jun 30 2021 at  4:12P -0400,
Martin Wilck <mwilck@suse.com> wrote:

> On Di, 2021-06-29 at 14:23 -0700, Keith Busch wrote:
> > On Tue, Jun 29, 2021 at 09:23:18PM +0200, Martin Wilck wrote:
> > > On Di, 2021-06-29 at 14:59 +0200, Christoph Hellwig wrote:
> > > > On Mon, Jun 28, 2021 at 04:57:33PM +0200, Martin Wilck wrote:
> > > > 
> > > > > The sg_io-on-multipath code needs to answer the question "is
> > > > > this a
> > > > > path or a target error?". Therefore it calls blk_path_error(),
> > > > > which
> > > > > requires obtaining a blk_status_t first. But that's not
> > > > > available
> > > > > in
> > > > > the sg_io code path. So how should I deal with this situation?
> > > > 
> > > > Not by exporting random crap from the SCSI code.

Helpful as always Christoph... ;)

> > > So, you'd prefer inlining scsi_result_to_blk_status()?
> > 
> > I don't think you need to. The only scsi_result_to_blk_status()
> > caller
> > is sg_io_to_blk_status(). That's already in the same file as
> > scsi_result_to_blk_status() so no need to export it. You could even
> > make
> > it static.
> 
> Thanks for your suggestion. I'd be lucky if this was true. But the most
> important users of scsi_result_to_blk_status() are in scsi_lib.c
> (scsi_io_completion_action(), scsi_io_completion_nz_result()).
> 
> If I move scsi_result_to_blk_status() to vmlinux without exporting it,
> it won't be available in the SCSI core any more, at least not with
> CONFIG_SCSI=m.

For what you're trying to accomplish with this patchset, you only need
sg_io_to_blk_status() exported.

So check with Martin and/or Bart on the best way to give
sg_io_to_blk_status() access to the equivalent of your
__scsi_result_to_blk_status() without exporting it.

I'd start by just folding patches 1 and 2, fixing up the logic Dan
Carpenter pointed ouit, and only exporting sg_io_to_blk_status().

Mike

WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@gmail.com>
To: Martin Wilck <mwilck@suse.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	linux-scsi@vger.kernel.org, Daniel Wagner <dwagner@suse.de>,
	emilne@redhat.com, linux-block@vger.kernel.org,
	dm-devel@redhat.com, Paolo Bonzini <pbonzini@redhat.com>,
	nkoenig@redhat.com, Bart Van Assche <Bart.VanAssche@sandisk.com>,
	Christoph Hellwig <hch@lst.de>,
	Alasdair G Kergon <agk@redhat.com>
Subject: Re: [dm-devel] [PATCH v4 1/3] scsi: scsi_ioctl: export __scsi_result_to_blk_status()
Date: Wed, 30 Jun 2021 12:01:45 -0400	[thread overview]
Message-ID: <YNyVafnX09cOIZPe@redhat.com> (raw)
In-Reply-To: <1aa1f875e7a85f9a331e88e4f8482588176bdb3a.camel@suse.com>

On Wed, Jun 30 2021 at  4:12P -0400,
Martin Wilck <mwilck@suse.com> wrote:

> On Di, 2021-06-29 at 14:23 -0700, Keith Busch wrote:
> > On Tue, Jun 29, 2021 at 09:23:18PM +0200, Martin Wilck wrote:
> > > On Di, 2021-06-29 at 14:59 +0200, Christoph Hellwig wrote:
> > > > On Mon, Jun 28, 2021 at 04:57:33PM +0200, Martin Wilck wrote:
> > > > 
> > > > > The sg_io-on-multipath code needs to answer the question "is
> > > > > this a
> > > > > path or a target error?". Therefore it calls blk_path_error(),
> > > > > which
> > > > > requires obtaining a blk_status_t first. But that's not
> > > > > available
> > > > > in
> > > > > the sg_io code path. So how should I deal with this situation?
> > > > 
> > > > Not by exporting random crap from the SCSI code.

Helpful as always Christoph... ;)

> > > So, you'd prefer inlining scsi_result_to_blk_status()?
> > 
> > I don't think you need to. The only scsi_result_to_blk_status()
> > caller
> > is sg_io_to_blk_status(). That's already in the same file as
> > scsi_result_to_blk_status() so no need to export it. You could even
> > make
> > it static.
> 
> Thanks for your suggestion. I'd be lucky if this was true. But the most
> important users of scsi_result_to_blk_status() are in scsi_lib.c
> (scsi_io_completion_action(), scsi_io_completion_nz_result()).
> 
> If I move scsi_result_to_blk_status() to vmlinux without exporting it,
> it won't be available in the SCSI core any more, at least not with
> CONFIG_SCSI=m.

For what you're trying to accomplish with this patchset, you only need
sg_io_to_blk_status() exported.

So check with Martin and/or Bart on the best way to give
sg_io_to_blk_status() access to the equivalent of your
__scsi_result_to_blk_status() without exporting it.

I'd start by just folding patches 1 and 2, fixing up the logic Dan
Carpenter pointed ouit, and only exporting sg_io_to_blk_status().

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


  reply	other threads:[~2021-06-30 16:01 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-28  9:52 [PATCH v4 0/3] scsi/dm: dm_blk_ioctl(): implement failover for SG_IO on dm-multipath mwilck
2021-06-28  9:52 ` [dm-devel] " mwilck
2021-06-28  9:52 ` [PATCH v4 1/3] scsi: scsi_ioctl: export __scsi_result_to_blk_status() mwilck
2021-06-28  9:52   ` [dm-devel] " mwilck
2021-06-28  9:53   ` Christoph Hellwig
2021-06-28  9:53     ` [dm-devel] " Christoph Hellwig
2021-06-28 14:57     ` Martin Wilck
2021-06-28 14:57       ` [dm-devel] " Martin Wilck
2021-06-29 12:59       ` Christoph Hellwig
2021-06-29 12:59         ` [dm-devel] " Christoph Hellwig
2021-06-29 19:23         ` Martin Wilck
2021-06-29 19:23           ` [dm-devel] " Martin Wilck
2021-06-29 21:23           ` Keith Busch
2021-06-29 21:23             ` [dm-devel] " Keith Busch
2021-06-30  8:12             ` Martin Wilck
2021-06-30  8:12               ` [dm-devel] " Martin Wilck
2021-06-30 16:01               ` Mike Snitzer [this message]
2021-06-30 16:01                 ` Mike Snitzer
2021-06-30 16:54                 ` Martin Wilck
2021-06-30 16:54                   ` [dm-devel] " Martin Wilck
2021-06-30 22:08                   ` Bart Van Assche
2021-06-30 22:08                     ` [dm-devel] " Bart Van Assche
2021-07-01  6:19                     ` Christoph Hellwig
2021-07-01  6:19                       ` [dm-devel] " Christoph Hellwig
2021-07-06 16:40                       ` Paolo Bonzini
2021-07-06 16:40                         ` [dm-devel] " Paolo Bonzini
2021-06-28  9:52 ` [PATCH v4 2/3] scsi: scsi_ioctl: add sg_io_to_blk_status() mwilck
2021-06-28  9:52   ` [dm-devel] " mwilck
2021-06-28 14:39   ` kernel test robot
2021-06-28 14:39     ` kernel test robot
2021-06-28 14:39     ` [dm-devel] " kernel test robot
2021-06-28  9:52 ` [PATCH v4 3/3] dm mpath: add CONFIG_DM_MULTIPATH_SG_IO - failover for SG_IO mwilck
2021-06-28  9:52   ` [dm-devel] " mwilck
2021-06-28 16:06 [PATCH v4 2/3] scsi: scsi_ioctl: add sg_io_to_blk_status() kernel test robot
2021-06-29  7:00 ` [kbuild] " Dan Carpenter
2021-06-29  7:00 ` [dm-devel] " Dan Carpenter
2021-06-29  7:00 ` Dan Carpenter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YNyVafnX09cOIZPe@redhat.com \
    --to=snitzer@gmail.com \
    --cc=Bart.VanAssche@sandisk.com \
    --cc=agk@redhat.com \
    --cc=bmarzins@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=dwagner@suse.de \
    --cc=emilne@redhat.com \
    --cc=hare@suse.de \
    --cc=hch@lst.de \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=mwilck@suse.com \
    --cc=nkoenig@redhat.com \
    --cc=pbonzini@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.