Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Linus Torvalds
On Tue, Dec 8, 2020 at 12:53 PM Al Viro wrote: > > Umm... I've got > fs: Handle I_DONTCACHE in iput_final() instead of generic_drop_inode() > and > fs: Kill DCACHE_DONTCACHE dentry even if DCACHE_REFERENCED is set > in "to apply" pile; if that's what you are talking about, Yup, those were the

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Al Viro
On Tue, Dec 08, 2020 at 12:25:55PM -0800, Linus Torvalds wrote: > On Tue, Dec 8, 2020 at 11:49 AM Al Viro wrote: > > > > Said that, it does appear to survive all beating, and it does fix > > a regression introduced in this cycle, so, provided that amount of > > comments in there is OK with you...

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Linus Torvalds
On Tue, Dec 8, 2020 at 11:49 AM Al Viro wrote: > > Said that, it does appear to survive all beating, and it does fix > a regression introduced in this cycle, so, provided that amount of > comments in there is OK with you... Ok, considering Greg's note, I've pulled it. It's early in the last

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Greg KH
On Tue, Dec 08, 2020 at 10:34:45AM -0800, Linus Torvalds wrote: > On Tue, Dec 8, 2020 at 8:35 AM Christoph Hellwig wrote: > > > > > > Shouldn't this go to Linus before v5.10 is released? > > > > ping? > > So by now I'm a bit worried about this series, because the early fixes > caused more

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Al Viro
On Tue, Dec 08, 2020 at 10:34:45AM -0800, Linus Torvalds wrote: > On Tue, Dec 8, 2020 at 8:35 AM Christoph Hellwig wrote: > > > > > > Shouldn't this go to Linus before v5.10 is released? > > > > ping? > > So by now I'm a bit worried about this series, because the early fixes > caused more

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Linus Torvalds
On Tue, Dec 8, 2020 at 8:35 AM Christoph Hellwig wrote: > > > > Shouldn't this go to Linus before v5.10 is released? > > ping? So by now I'm a bit worried about this series, because the early fixes caused more problems than the current state. So considering the timing and Al having been spotty,

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-12-08 Thread Christoph Hellwig
On Fri, Nov 27, 2020 at 05:29:02PM +0100, Christoph Hellwig wrote: > On Mon, Nov 16, 2020 at 03:29:42AM +, Al Viro wrote: > > > Still good. > > > > > > Tested-by: Nathan Chancellor > > > > Pushed into #fixes > > Shouldn't this go to Linus before v5.10 is released? ping?

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-27 Thread Christoph Hellwig
On Mon, Nov 16, 2020 at 03:29:42AM +, Al Viro wrote: > > Still good. > > > > Tested-by: Nathan Chancellor > > Pushed into #fixes Shouldn't this go to Linus before v5.10 is released?

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Al Viro
On Sun, Nov 15, 2020 at 05:34:16PM -0700, Nathan Chancellor wrote: > Still good. > > Tested-by: Nathan Chancellor Pushed into #fixes > > BTW, is that call of readv() really coming from init? And if it > > is, what version of init are you using? > > I believe that it is but since this is

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Nathan Chancellor
On Mon, Nov 16, 2020 at 12:25:13AM +, Al Viro wrote: > On Sun, Nov 15, 2020 at 04:51:49PM -0700, Nathan Chancellor wrote: > > Looks good to me on top of d4d50710a8b46082224376ef119a4dbb75b25c56, > > thanks for quickly looking into this! > > > > Tested-by: Nathan Chancellor > > OK... a

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Al Viro
On Sun, Nov 15, 2020 at 04:51:49PM -0700, Nathan Chancellor wrote: > Looks good to me on top of d4d50710a8b46082224376ef119a4dbb75b25c56, > thanks for quickly looking into this! > > Tested-by: Nathan Chancellor OK... a variant with (hopefully) better comments and cleaned up logics in the second

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Nathan Chancellor
On Sun, Nov 15, 2020 at 11:38:14PM +, Al Viro wrote: > On Sun, Nov 15, 2020 at 02:41:25PM -0700, Nathan Chancellor wrote: > > Hi Al, > > > > Apologies for the delay. > > > > On Sun, Nov 15, 2020 at 03:53:55PM +, Al Viro wrote: > > > On Sat, Nov 14, 2020 at 08:50:00PM +, Al Viro

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Al Viro
On Sun, Nov 15, 2020 at 02:41:25PM -0700, Nathan Chancellor wrote: > Hi Al, > > Apologies for the delay. > > On Sun, Nov 15, 2020 at 03:53:55PM +, Al Viro wrote: > > On Sat, Nov 14, 2020 at 08:50:00PM +, Al Viro wrote: > > > > OK, I think I understand what's going on. Could you check

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Linus Torvalds
On Sun, Nov 15, 2020 at 7:54 AM Al Viro wrote: > > OK, I think I understand what's going on. Could you check if > reverting the variant in -next and applying the following instead > fixes what you are seeing? Side note: if this ends up working, can you add a lot of comments about this thing

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-15 Thread Al Viro
On Sat, Nov 14, 2020 at 08:50:00PM +, Al Viro wrote: OK, I think I understand what's going on. Could you check if reverting the variant in -next and applying the following instead fixes what you are seeing? diff --git a/fs/seq_file.c b/fs/seq_file.c index 3b20e21604e7..35667112bbd1 100644

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-14 Thread Al Viro
On Fri, Nov 13, 2020 at 04:54:53PM -0700, Nathan Chancellor wrote: > Hi Al, > > On Wed, Nov 11, 2020 at 10:21:16PM +, Al Viro wrote: > > On Wed, Nov 11, 2020 at 09:52:20PM +, Al Viro wrote: > > > > > That can be done, but I would rather go with > > > n = copy_to_iter(m->buf +

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-14 Thread Al Viro
On Sat, Nov 14, 2020 at 07:00:25AM +, Al Viro wrote: > On Fri, Nov 13, 2020 at 11:19:34PM -0700, Nathan Chancellor wrote: > > > Assuming so, I have attached the output both with and without the > > WARN_ON. Looks like mountinfo is what is causing the error? > > Cute... FWIW, on #origin +

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Al Viro
On Fri, Nov 13, 2020 at 11:19:34PM -0700, Nathan Chancellor wrote: > Assuming so, I have attached the output both with and without the > WARN_ON. Looks like mountinfo is what is causing the error? Cute... FWIW, on #origin + that commit with fix folded in I don't see anything unusual in reads

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Al Viro
On Fri, Nov 13, 2020 at 09:14:20PM -0700, Nathan Chancellor wrote: > Unfortunately that patch does not solve my issue. Is there any other > debugging I should add? Hmm... I wonder which file it is; how about if (WARN_ON(!iovec.iov_len)) printk(KERN_ERR

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Nathan Chancellor
On Sat, Nov 14, 2020 at 03:54:53AM +, Al Viro wrote: > On Fri, Nov 13, 2020 at 08:01:24PM -0700, Nathan Chancellor wrote: > > Sure thing, it does trigger. > > > > [0.235058] [ cut here ] > > [0.235062] WARNING: CPU: 15 PID: 237 at fs/seq_file.c:176 > >

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Al Viro
On Fri, Nov 13, 2020 at 08:01:24PM -0700, Nathan Chancellor wrote: > Sure thing, it does trigger. > > [0.235058] [ cut here ] > [0.235062] WARNING: CPU: 15 PID: 237 at fs/seq_file.c:176 > seq_read_iter+0x3b3/0x3f0 > [0.235064] CPU: 15 PID: 237 Comm: localhost

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Nathan Chancellor
On Sat, Nov 14, 2020 at 01:17:54AM +, Al Viro wrote: > On Fri, Nov 13, 2020 at 04:54:53PM -0700, Nathan Chancellor wrote: > > > This patch in -next (6a9f696d1627bacc91d1cebcfb177f474484e8ba) breaks > > WSL2's interoperability feature, where Windows paths automatically get > > added to PATH on

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Al Viro
On Fri, Nov 13, 2020 at 04:54:53PM -0700, Nathan Chancellor wrote: > This patch in -next (6a9f696d1627bacc91d1cebcfb177f474484e8ba) breaks > WSL2's interoperability feature, where Windows paths automatically get > added to PATH on start up so that Windows binaries can be accessed from > within

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-13 Thread Nathan Chancellor
Hi Al, On Wed, Nov 11, 2020 at 10:21:16PM +, Al Viro wrote: > On Wed, Nov 11, 2020 at 09:52:20PM +, Al Viro wrote: > > > That can be done, but I would rather go with > > n = copy_to_iter(m->buf + m->from, m->count, iter); > > m->count -= n; > > m->from

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-11 Thread Linus Torvalds
On Wed, Nov 11, 2020 at 2:21 PM Al Viro wrote: > > Something like below (build-tested only): Apart from my usual "oh, Gods, the iter model really does confuse me" this looks more like what I expected, yes. Considering the original bug, I'm clearly not the only one confused by the iov_iter

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-11 Thread Al Viro
On Wed, Nov 11, 2020 at 02:27:02PM -0800, Linus Torvalds wrote: > On Wed, Nov 11, 2020 at 2:21 PM Al Viro wrote: > > > > Something like below (build-tested only): > > Apart from my usual "oh, Gods, the iter model really does confuse me" > this looks more like what I expected, yes. > >

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-11 Thread Al Viro
On Wed, Nov 11, 2020 at 09:52:20PM +, Al Viro wrote: > That can be done, but I would rather go with > n = copy_to_iter(m->buf + m->from, m->count, iter); > m->count -= n; > m->from += n; > copied += n; > if (!size) >

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-11 Thread Al Viro
On Wed, Nov 11, 2020 at 09:54:12AM -0800, Linus Torvalds wrote: > On Tue, Nov 10, 2020 at 3:20 PM Al Viro wrote: > > > > Any objections to the following? > > Well, I don't _object_, but I find it ugly. > > And I think both the old and the "fixed" code is wrong when an EFAULT > happens in the

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-11 Thread Linus Torvalds
On Tue, Nov 10, 2020 at 3:20 PM Al Viro wrote: > > Any objections to the following? Well, I don't _object_, but I find it ugly. And I think both the old and the "fixed" code is wrong when an EFAULT happens in the middle. Yes, we can just return EFAULT. But for read() and write() we really try

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-10 Thread Christoph Hellwig
On Tue, Nov 10, 2020 at 11:20:28PM +, Al Viro wrote: > On Tue, Nov 10, 2020 at 09:35:11PM +, Al Viro wrote: > > On Tue, Nov 10, 2020 at 09:32:53PM +, Al Viro wrote: > > > > > AFAICS, not all callers want that semantics, but I think it's worth > > > a new primitive. I'm not saying it

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-10 Thread Al Viro
On Tue, Nov 10, 2020 at 09:35:11PM +, Al Viro wrote: > On Tue, Nov 10, 2020 at 09:32:53PM +, Al Viro wrote: > > > AFAICS, not all callers want that semantics, but I think it's worth > > a new primitive. I'm not saying it should be a prereq for your > > series, but either that or an

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-10 Thread Al Viro
On Tue, Nov 10, 2020 at 09:32:53PM +, Al Viro wrote: > AFAICS, not all callers want that semantics, but I think it's worth > a new primitive. I'm not saying it should be a prereq for your > series, but either that or an explicit iov_iter_revert() is needed. Seeing that it already went into

Re: [PATCH 1/6] seq_file: add seq_read_iter

2020-11-10 Thread Al Viro
On Wed, Nov 04, 2020 at 09:27:33AM +0100, Christoph Hellwig wrote: > ssize_t seq_read(struct file *file, char __user *buf, size_t size, loff_t > *ppos) > { > - struct seq_file *m = file->private_data; > + struct iovec iov = { .iov_base = buf, .iov_len = size}; > + struct kiocb