Re: [PATCH vfs.all 15/26] s390/dasd: use bdev api in dasd_format()

[Stefan Haberland <sth@linux.ibm.com>]

Am 16.04.24 um 10:47 schrieb Alexander Gordeev:
> On Tue, Apr 16, 2024 at 02:35:55AM +0100, Al Viro wrote:
>>>   drivers/s390/block/dasd_ioctl.c | 5 +++--
>>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/s390/block/dasd_ioctl.c b/drivers/s390/block/dasd_ioctl.c
>>> index 7e0ed7032f76..c1201590f343 100644
>>> --- a/drivers/s390/block/dasd_ioctl.c
>>> +++ b/drivers/s390/block/dasd_ioctl.c
>>> @@ -215,8 +215,9 @@ dasd_format(struct dasd_block *block, struct format_data_t *fdata)
>>>   	 * enabling the device later.
>>>   	 */
>>>   	if (fdata->start_unit == 0) {
>>> -		block->gdp->part0->bd_inode->i_blkbits =
>>> -			blksize_bits(fdata->blksize);
>>> +		rc = set_blocksize(block->gdp->part0, fdata->blksize);
>> Could somebody (preferably s390 folks) explain what is going on in
>> dasd_format()?  The change in this commit is *NOT* an equivalent
>> transformation - mainline does not evict the page cache of device.
>>
>> Is that
>> 	* intentional behaviour in mainline version, possibly broken
>> by this patch
>> 	* a bug in mainline accidentally fixed by this patch
>> 	* something else?
>>
>> And shouldn't there be an exclusion between that and having a filesystem
>> on a partition of that disk currently mounted?
> CC-ing Stefan and Jan.
>
> Thanks!

Hi,
from my point of view this was an equivalent transformation.

set_blocksize() does basically also set i_blkbits like it was before.
The dasd_format ioctl does only work on a disabled device. To achieve this
all partitions need to be unmounted.
The tooling also refuses to work on disks actually in use.

So there should be no page cache to evict.

The comment above this code says:

/* Since dasdfmt keeps the device open after it was disabled,
  * there still exists an inode for this device.
  * We must update i_blkbits, otherwise we might get errors when
  * enabling the device later.
  */

This is the reason for updating i_blkbits.

However, I get your point to question the code itself.

Honestly this code exists for many years and I can not tell if the
circumstances of the comment have changed in between somehow.
A quick test without this code did not show any change or errors but
there might be corner cases I am missing.

Maybe you can give a hint if this makes any sense from your point of view.

Thanks,
Stefan