On Fri, 7 Feb 2020 14:13:05 +0100 Christian Borntraeger <borntraeger@xxxxxxxxxx> 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@xxxxxxxxxxxxx> > > --- > > 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. This is definitely an improvement. > > but this code is still fishy: I'm surprised it took that long; it's been 14 years since I messed up^W^Wwrote this and there's basically only been a memory leak fix from you in the meantime... that said, ... > > $ 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 ...what we are doing is translating something that is basically a per-possible-device value into a range, as otherwise the output would be quite unreadable for humans. I'm not sure what the semantics should be if you read in small chunks etc., as the ranges are assembled on-the-fly. > Peter, any opinions on this? I *think* I originally modeled /proc/cio_ignore on a long-gone dasd procfs file (a very long time before converting it to a seq file); do we have any other examples of files that do a similar individual-values-to-ranges conversion?
