All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jisheng Zhang <Jisheng.Zhang@synaptics.com>
To: Dominique Martinet <asmadeus@codewreck.org>
Cc: Eric Van Hensbergen <ericvh@gmail.com>,
	Latchesar Ionkov <lucho@ionkov.net>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	v9fs-developer@lists.sourceforge.net, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] net: 9p: free what was emitted when read count is 0
Date: Mon, 1 Mar 2021 11:01:57 +0800	[thread overview]
Message-ID: <20210301110157.19d9ad4e@xhacker.debian> (raw)
In-Reply-To: <YDxWrB8AoxJOmScE@odin>

On Mon, 1 Mar 2021 11:51:24 +0900 Dominique Martinet wrote:


> 
> 
> Jisheng Zhang wrote on Mon, Mar 01, 2021 at 10:33:36AM +0800:
> > I met below warning when cating a small size(about 80bytes) txt file
> > on 9pfs(msize=2097152 is passed to 9p mount option), the reason is we
> > miss iov_iter_advance() if the read count is 0, so we didn't truncate
> > the pipe, then iov_iter_pipe() thinks the pipe is full. Fix it by
> > calling iov_iter_advance() on the iov_iter "to" even if read count is 0  
> 
> Hm, there are plenty of other error cases that don't call
> iov_iter_advance() and shouldn't trigger this warning ; I'm not sure
> just adding one particular call to this is a good solution.

Per my understanding of iov_iter, we need to call iov_iter_advance()
even when the read out count is 0. I believe we can see this common style
in other fs.

> 
> 
> How reproducible is this? From the description it should happen

100%

> everytime you cat a small file? (I'm surprised cat uses sendfile, what

it happened every time when catting a small file.

> cat version? coreutils' doesn't seem to do that on their git)

busybox cat

> 
> What kernel version do you get this on? Bonus points if you can confirm

5.11 and the latest linus tree

> this didn't use to happen, and full points for a bisect.
> 
> 
> (cat on a small file is something I do all the time in my tests, I'd
> like to be able to reproduce to understand the issue better as I'm not
> familiar with that part of the code)

Per my check, it can be 100% reproduced with busybox cat + "msize=2097152"
mount option. NOTE: msize=2097152 isn't a magic number it can be other
numbers which can ensure zerocopy code path is executed: p9_client_read_once
->p9_client_zc_rpc()

Thanks

  reply	other threads:[~2021-03-01  3:03 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-01  2:33 [PATCH] net: 9p: free what was emitted when read count is 0 Jisheng Zhang
2021-03-01  2:51 ` Dominique Martinet
2021-03-01  3:01   ` Jisheng Zhang [this message]
2021-03-02  4:38     ` Dominique Martinet
2021-03-02  7:39       ` Jisheng Zhang
2021-03-02  8:08         ` Dominique Martinet
2021-03-02  9:22           ` Jisheng Zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210301110157.19d9ad4e@xhacker.debian \
    --to=jisheng.zhang@synaptics.com \
    --cc=asmadeus@codewreck.org \
    --cc=davem@davemloft.net \
    --cc=ericvh@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lucho@ionkov.net \
    --cc=netdev@vger.kernel.org \
    --cc=v9fs-developer@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.