Re: [PATCH 1/1] cio_ignore_proc_seq_next should increase position index

[Peter Oberparleiter <oberpar@linux.ibm.com>]

On 07.02.2020 14:13, Christian Borntraeger wrote:
> 
> 
> On 24.01.20 06:48, Vasily Averin wrote:
>> if seq_file .next fuction does not change position index,
>> read after some lseek can generate unexpected output.
>>
>> https://bugzilla.kernel.org/show_bug.cgi?id=206283
>> Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
>> ---
>>  drivers/s390/cio/blacklist.c | 5 +++--
>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/s390/cio/blacklist.c b/drivers/s390/cio/blacklist.c
>> index 2a3f874..9cebff8 100644
>> --- a/drivers/s390/cio/blacklist.c
>> +++ b/drivers/s390/cio/blacklist.c
>> @@ -303,8 +303,10 @@ struct ccwdev_iter {
>>  cio_ignore_proc_seq_next(struct seq_file *s, void *it, loff_t *offset)
>>  {
>>  	struct ccwdev_iter *iter;
>> +	loff_t p = *offset;
>>  
>> -	if (*offset >= (__MAX_SUBCHANNEL + 1) * (__MAX_SSID + 1))
>> +	(*offset)++;
>> +	if (p >= (__MAX_SUBCHANNEL + 1) * (__MAX_SSID + 1))
>>  		return NULL;
>>  	iter = it;
>>  	if (iter->devno == __MAX_SUBCHANNEL) {
>> @@ -314,7 +316,6 @@ struct ccwdev_iter {
>>  			return NULL;
>>  	} else
>>  		iter->devno++;
>> -	(*offset)++;
>>  	return iter;
>>  }
>>  
>>
> 
> I guess this fixes one aspect:
> "dd: /proc/cio_ignore: cannot skip to specified offset"
> is now gone. So I am tempted to apply this. 
> 
> but this code is still fishy:
> 
> $ cat /proc/cio_ignore 
> 0.0.fe00-0.0.fefe
> 0.0.ff00-0.0.ffff
> $ dd if=/proc/cio_ignore status=none
> 0.0.fe00-0.0.fefe
> 0.0.ff00-0.0.ffff
> $ dd if=/proc/cio_ignore status=none bs=10
> 0.0.fe00-0.0.fefe
> 0.0.ff00-0.0.ff01-0.0.ff02-0.0.ff03-0.0.ff04-0.0.ff05-0.0.ff06-0.0.ff07-0.0.ff08-0.0.ffff
> $ dd if=/proc/cio_ignore status=none bs=10 skip=1
> .0.fefe
> 0.0.ff00-0.0.ff01-0.0.ff02-0.0.ff03-0.0.ff04-0.0.ff05-0.0.ff06-0.0.ff07-0.0.ff08-0.0.ffff
> 
> 
> Peter, any opinions on this?

A correct implementation of a file read operation must result in the
same data being read independently of whether the file is read in one
go, or if it is read byte-by-byte.

It seems that the current cio_ignore seq-file implementation doesn't
meet that requirement. I don't think that this patch series is the best
way to address this problem though.

My suggestion would be to apply this patch set as is, and then I'll take
the to-do to fix this seq file implementation at a later time.

-- 
Peter Oberparleiter
Linux on Z Development - IBM Germany