All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
To: John Garry <john.garry@huawei.com>,
	Christoph Hellwig <hch@infradead.org>
Cc: "Ewan D. Milne" <emilne@redhat.com>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Alim Akhtar <alim.akhtar@samsung.com>,
	Avri Altman <avri.altman@wdc.com>,
	Doug Gilbert <dgilbert@interlog.com>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org,
	james.smart@broadcom.com
Subject: Re: [PATCH 1/4] scsi: core: constify pointer to scsi_host_template
Date: Mon, 9 May 2022 15:20:48 +0200	[thread overview]
Message-ID: <d01c29c1-bb5a-281d-ef71-9c7b39e28d23@linaro.org> (raw)
In-Reply-To: <4510c5dc-3d7d-fc5f-cb80-34da7dbaaa8e@huawei.com>

On 09/05/2022 13:28, John Garry wrote:
> 
> For some reason I cannot fetch your git due to "error: RPC failed ..." 
> which I think is a timeout. I seem to have this problem recently 
> whenever a linux.git clone has branches based on linux-next.git . Maybe 
> a git config issue for me...

Just to be sure - the link was not a git remote, but direct link for a
web browser. The repo is:
https://github.com/krzk/linux.git
branch: n/qcom-ufs-opp-cleanups-v2

>> However this does not solve the problem. The SHT has "module" which gets
>> incremented/decremented. Exactly like in case of other drivers
>> (driver->owner).
> 
> Ah, I missed that this could be a problem. So we have this member to 
> stop the SCSI host driver being removed when we have disks mounted, etc.
> 
> But isn't scsi_host_template.module just a pointer to the local driver 
> module data (and that data gets incremented/decremented)? I am looking 
> at the THIS_MODULE definition in export.h:

Yes, it is just a pointer, just like 'struct device_driver.owner' is.

> 
> extern stuct module __this_module;
> #define THIS_MODULE(&__this_module)
> 
> However I do see scsi_device_dev_release(), which does:
> 
> sdp->host->hostt->module = NULL
> 
> I am not sure how necessary that really is. I would need to check further.

> 
> Did you see if there other places which set hostt->module dynamically?

I think all SHT set it statically. Then it is being dynamically
incremented when needed and in scsi_device_dev_release() is being
nullified (as you mentioned above).

I guess this SHT->module is actually duplicating what most of drivers
(e.g. PCI, platform, USB) are doing. If I understand correctly, the
Scsi_Host is instantiated by the probe of a driver (pci_driver,
virtio_driver etc), therefore the SHT->module could be simply stored in
Scsi_Host.


>>
>> I started moving the SHT->module to a new field scsi_host->owner and
>> trying to use the parent's driver (so PCI, USB) owner.
>> I am not sure if it is correct approach, so before implementing such big
>> change affecting multiple subsystems (USB, ATA, SCSI) - can you share
>> ideas/opinion?
>>
>> The Work-in-Progress looks like this (last commit):
>> https://github.com/krzk/linux/commit/17609caecd53df20f631703ea084a70e7735b5d7



Best regards,
Krzysztof

  reply	other threads:[~2022-05-09 13:21 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-04-08 10:30 [PATCH 1/4] scsi: core: constify pointer to scsi_host_template Krzysztof Kozlowski
2022-04-08 10:30 ` [PATCH 2/4] scsi: core: fix white-spaces Krzysztof Kozlowski
2022-05-04  8:47   ` Krzysztof Kozlowski
2022-04-08 10:30 ` [PATCH 3/4] scsi: ufs: ufshcd-pltfrm: constify pointed data Krzysztof Kozlowski
2022-04-08 10:30 ` [PATCH 4/4] scsi: ufs: ufshcd: " Krzysztof Kozlowski
2022-04-08 14:35   ` Bart Van Assche
2022-04-08 12:14 ` [PATCH 1/4] scsi: core: constify pointer to scsi_host_template John Garry
2022-04-08 12:32   ` Krzysztof Kozlowski
2022-04-08 12:57     ` John Garry
2022-04-08 19:31       ` Ewan D. Milne
2022-04-12  7:57         ` John Garry
2022-04-20  7:03           ` Christoph Hellwig
2022-04-25  8:58             ` John Garry
2022-04-25  9:22               ` Krzysztof Kozlowski
2022-04-25 13:04                 ` John Garry
2022-04-26  1:16                   ` Bart Van Assche
2022-04-26  1:54                     ` Douglas Gilbert
2022-04-26  4:13                       ` Bart Van Assche
2022-04-27  1:47                         ` Douglas Gilbert
2022-05-06 16:42                   ` Krzysztof Kozlowski
2022-05-09 11:28                     ` John Garry
2022-05-09 13:20                       ` Krzysztof Kozlowski [this message]
2022-05-09 14:50                         ` John Garry
2022-05-11  8:31                           ` Christoph Hellwig

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=d01c29c1-bb5a-281d-ef71-9c7b39e28d23@linaro.org \
    --to=krzysztof.kozlowski@linaro.org \
    --cc=alim.akhtar@samsung.com \
    --cc=avri.altman@wdc.com \
    --cc=dgilbert@interlog.com \
    --cc=emilne@redhat.com \
    --cc=hch@infradead.org \
    --cc=james.smart@broadcom.com \
    --cc=jejb@linux.ibm.com \
    --cc=john.garry@huawei.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.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.