2019-10-14 13:53:42

by Zhangshaokun

[permalink] [raw]
Subject: [PATCH] docs: block: Remove blk_init_queue related description

blk_init_queue has been removed since commit <a1ce35fa4985>
("block: remove dead elevator code"), Let's cleanup the description
in the biodoc.rst document.

Cc: Jonathan Corbet <[email protected]>
Cc: Jens Axboe <[email protected]>
Signed-off-by: Shaokun Zhang <[email protected]>
---
Documentation/block/biodoc.rst | 10 ----------
1 file changed, 10 deletions(-)

diff --git a/Documentation/block/biodoc.rst b/Documentation/block/biodoc.rst
index b964796ec9c7..a19081d88349 100644
--- a/Documentation/block/biodoc.rst
+++ b/Documentation/block/biodoc.rst
@@ -1013,11 +1013,6 @@ request_fn execution which it means that lots of older drivers
should still be SMP safe. Drivers are free to drop the queue
lock themselves, if required. Drivers that explicitly used the
io_request_lock for serialization need to be modified accordingly.
-Usually it's as easy as adding a global lock::
-
- static DEFINE_SPINLOCK(my_driver_lock);
-
-and passing the address to that lock to blk_init_queue().

5.2 64 bit sector numbers (sector_t prepares for 64 bit support)
----------------------------------------------------------------
@@ -1071,11 +1066,6 @@ right thing to use is bio_endio(bio) instead.
If the driver is dropping the io_request_lock from its request_fn strategy,
then it just needs to replace that with q->queue_lock instead.

-As described in Sec 1.1, drivers can set max sector size, max segment size
-etc per queue now. Drivers that used to define their own merge functions i
-to handle things like this can now just use the blk_queue_* functions at
-blk_init_queue time.
-
Drivers no longer have to map a {partition, sector offset} into the
correct absolute location anymore, this is done by the block layer, so
where a driver received a request ala this before::
--
2.7.4


2019-10-19 08:35:05

by Jonathan Corbet

[permalink] [raw]
Subject: Re: [PATCH] docs: block: Remove blk_init_queue related description

On Mon, 14 Oct 2019 21:50:02 +0800
Shaokun Zhang <[email protected]> wrote:

> blk_init_queue has been removed since commit <a1ce35fa4985>
> ("block: remove dead elevator code"), Let's cleanup the description
> in the biodoc.rst document.
>
> Cc: Jonathan Corbet <[email protected]>
> Cc: Jens Axboe <[email protected]>
> Signed-off-by: Shaokun Zhang <[email protected]>

So I applied this, then changed my mind and unapplied it; I think it's the
wrong fix.

> Documentation/block/biodoc.rst | 10 ----------
> 1 file changed, 10 deletions(-)
>
> diff --git a/Documentation/block/biodoc.rst b/Documentation/block/biodoc.rst
> index b964796ec9c7..a19081d88349 100644
> --- a/Documentation/block/biodoc.rst
> +++ b/Documentation/block/biodoc.rst
> @@ -1013,11 +1013,6 @@ request_fn execution which it means that lots of older drivers
> should still be SMP safe. Drivers are free to drop the queue
> lock themselves, if required. Drivers that explicitly used the
> io_request_lock for serialization need to be modified accordingly.
> -Usually it's as easy as adding a global lock::
> -
> - static DEFINE_SPINLOCK(my_driver_lock);
> -
> -and passing the address to that lock to blk_init_queue().

This is a section about coping with the removal of the io_request_lock,
which happened in 2.5, prior to the git era. I think it is probably safe
to say that there are no relevant drivers that still need to be updated
for this particular change.

> 5.2 64 bit sector numbers (sector_t prepares for 64 bit support)
> ----------------------------------------------------------------
> @@ -1071,11 +1066,6 @@ right thing to use is bio_endio(bio) instead.
> If the driver is dropping the io_request_lock from its request_fn strategy,
> then it just needs to replace that with q->queue_lock instead.
>
> -As described in Sec 1.1, drivers can set max sector size, max segment size
> -etc per queue now. Drivers that used to define their own merge functions i
> -to handle things like this can now just use the blk_queue_* functions at
> -blk_init_queue time.
> -
> Drivers no longer have to map a {partition, sector offset} into the
> correct absolute location anymore, this is done by the block layer, so
> where a driver received a request ala this before::

Here, too. We're talking about teaching drivers how to use bios.

My suggested fix is to just remove both sections from the document
entirely; neither is relevant in 2019.

Even better, of course, would be to pass through this document and bring
it up to current practice in general; there is certain to be a lot more in
need of fixing here.

Thanks,

jon

2019-10-21 08:44:15

by Zhangshaokun

[permalink] [raw]
Subject: Re: [PATCH] docs: block: Remove blk_init_queue related description

Hi Jonathan,

On 2019/10/18 23:39, Jonathan Corbet wrote:
> On Mon, 14 Oct 2019 21:50:02 +0800
> Shaokun Zhang <[email protected]> wrote:
>
>> blk_init_queue has been removed since commit <a1ce35fa4985>
>> ("block: remove dead elevator code"), Let's cleanup the description
>> in the biodoc.rst document.
>>
>> Cc: Jonathan Corbet <[email protected]>
>> Cc: Jens Axboe <[email protected]>
>> Signed-off-by: Shaokun Zhang <[email protected]>
>
> So I applied this, then changed my mind and unapplied it; I think it's the
> wrong fix.
>

Thanks your reply.

Let's to introduce why I hit this: I have a module driver that can works using
4.19 kernel version, it breaks when I use the 5.3 kernel. While I checked that
blk_init_queue has been removed by commit <a1ce35fa4985>
("block: remove dead elevator code"), but there is still something about it in
this document.

>> Documentation/block/biodoc.rst | 10 ----------
>> 1 file changed, 10 deletions(-)
>>
>> diff --git a/Documentation/block/biodoc.rst b/Documentation/block/biodoc.rst
>> index b964796ec9c7..a19081d88349 100644
>> --- a/Documentation/block/biodoc.rst
>> +++ b/Documentation/block/biodoc.rst
>> @@ -1013,11 +1013,6 @@ request_fn execution which it means that lots of older drivers
>> should still be SMP safe. Drivers are free to drop the queue
>> lock themselves, if required. Drivers that explicitly used the
>> io_request_lock for serialization need to be modified accordingly.
>> -Usually it's as easy as adding a global lock::
>> -
>> - static DEFINE_SPINLOCK(my_driver_lock);
>> -
>> -and passing the address to that lock to blk_init_queue().
>
> This is a section about coping with the removal of the io_request_lock,
> which happened in 2.5, prior to the git era. I think it is probably safe
> to say that there are no relevant drivers that still need to be updated
> for this particular change.
>
>> 5.2 64 bit sector numbers (sector_t prepares for 64 bit support)
>> ----------------------------------------------------------------
>> @@ -1071,11 +1066,6 @@ right thing to use is bio_endio(bio) instead.
>> If the driver is dropping the io_request_lock from its request_fn strategy,
>> then it just needs to replace that with q->queue_lock instead.
>>
>> -As described in Sec 1.1, drivers can set max sector size, max segment size
>> -etc per queue now. Drivers that used to define their own merge functions i
>> -to handle things like this can now just use the blk_queue_* functions at
>> -blk_init_queue time.
>> -
>> Drivers no longer have to map a {partition, sector offset} into the
>> correct absolute location anymore, this is done by the block layer, so
>> where a driver received a request ala this before::
>
> Here, too. We're talking about teaching drivers how to use bios.
>
> My suggested fix is to just remove both sections from the document
> entirely; neither is relevant in 2019.
>
> Even better, of course, would be to pass through this document and bring
> it up to current practice in general; there is certain to be a lot more in
> need of fixing here.
>

I'm not very familiar with bio driver and this document, So is Jens or anyone happy to
do more fixing about it.

Thanks,
Shaokun

> Thanks,
>
> jon
>
> .
>