linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Martin K. Petersen" <martin.petersen@oracle.com>
To: John Garry <john.garry@huawei.com>
Cc: "Martin K. Petersen" <martin.petersen@oracle.com>,
	Christoph Hellwig <hch@lst.de>,
	Logan Gunthorpe <logang@deltatee.com>,
	<linux-scsi@vger.kernel.org>, <linux-block@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>,
	Intel SCU Linux support <intel-linux-scu@intel.com>,
	Artur Paszkiewicz <artur.paszkiewicz@intel.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	Jens Axboe <axboe@kernel.dk>, Jeff Moyer <jmoyer@redhat.com>,
	chenxiang <chenxiang66@hisilicon.com>
Subject: Re: [PATCH] scsi: isci: initialize shost fully before calling scsi_add_host()
Date: Tue, 15 Jan 2019 21:54:01 -0500	[thread overview]
Message-ID: <yq1sgxts46u.fsf@oracle.com> (raw)
In-Reply-To: <0ffaf166-c7e5-b135-fdc5-bcac24148e62@huawei.com> (John Garry's message of "Mon, 14 Jan 2019 12:10:23 +0000")


Hi John,

>> So in this case I think that accessor functions are actually better
>> because they allow us to print a big fat warning when you twiddle
>> something you shouldn't post-initialization. So that's something I think
>> we could--and should--improve.
>>
> Sure, this is an alternative, but I would rather make it obvious when
> these parameters should be set so that this would not be required.

I would like to have a mechanism in place that warns if you twiddle
things that break assumptions made at host registration time. That's not
a scenario the old registration interface was designed to handle.

I am not sure I agree with your assertion that setting these masks in
the struct prior to scsi_add_host() makes this ordering requirement much
more obvious. It is not like you are passing in a list of parameters and
then receiving a separately instantiated immutable scsi_host object. You
are performing an operation on something you already have and own.

That's why I commented that the current intersection between
compile-time static host template, dynamically discovered
pre-registration scsi_host parameters, and the registered runtime
scsi_host struct is somewhat messy.

Btw. I have no attachment to the prot wrappers whatsoever. The reason
they exist is that the SCSI integrity support was optional. And
therefore we had stub functions so things could be compiled without any
of the integrity fields being present in the various SCSI structs. So I
don't have any problem killing the wrappers except they may actually be
handy for regressions like this one where you could #error if the driver
writer violates the ordering requirement.

-- 
Martin K. Petersen	Oracle Linux Engineering

  reply	other threads:[~2019-01-16  2:54 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-08 20:50 [PATCH] scsi: isci: initialize shost fully before calling scsi_add_host() Logan Gunthorpe
2019-01-08 21:25 ` Jeff Moyer
2019-01-08 21:30 ` Jens Axboe
2019-01-09  3:29 ` Martin K. Petersen
2019-01-09 18:41 ` Christoph Hellwig
2019-01-10  9:11   ` John Garry
2019-01-12  2:34     ` Martin K. Petersen
2019-01-14 12:10       ` John Garry
2019-01-16  2:54         ` Martin K. Petersen [this message]
2019-01-16 14:44           ` John Garry

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=yq1sgxts46u.fsf@oracle.com \
    --to=martin.petersen@oracle.com \
    --cc=artur.paszkiewicz@intel.com \
    --cc=axboe@kernel.dk \
    --cc=chenxiang66@hisilicon.com \
    --cc=hch@lst.de \
    --cc=intel-linux-scu@intel.com \
    --cc=jejb@linux.ibm.com \
    --cc=jmoyer@redhat.com \
    --cc=john.garry@huawei.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=logang@deltatee.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).