Re: [PATCH 3/4] fuse: writepage: update bdi writeout when deletingsecondary request

[Maxim Patlasov]

On 10/03/2013 02:26 PM, Miklos Szeredi wrote:
> On Wed, Oct 02, 2013 at 09:38:43PM +0400, Maxim Patlasov wrote:
> > BDI_WRITTEN counter is used to estimate bdi bandwidth. It must be incremented
> > every time as bdi ends page writeback. No matter whether it was fulfilled by
> > actual write or by discarding the request (e.g. due to shrunk i_size).
> >
> > Note that even before writepages patches, the case "Got truncated off
> > completely" was handled in fuse_send_writepage() by calling
> > fuse_writepage_finish() which updated BDI_WRITTEN unconditionally.
> Hmm, I'm not sure I can agree with this.  If BDI_WRITTEN is used for bandwidth
> estimation, then I think it's more correct not to count rewrites and truncated
> pages.

I thought about it before submitting the patch, but my understanding is 
a bit different. Look how balance_dirty_pages and friends juggle with 
BDI_WRITTEN and BDI_DIRTIED. That layer knows nothing about fuse and its 
internals. Imagine that right now (if actual backend throughput is about 
10MB/sec) you believe that dirtying 26 pages per 10 milliseconds is 
fine, but when they lapsed you discovers that BDI_DIRTIED delta is 26 
while BDI_WRITTEN delta is only 13. Logically, you must decide to cut 
dirty-rate by factor two, but the decision would be incorrect in case of 
unaccounted truncated rewrites.

> But I don't see this matter either way since this is just used as a heuristic
> and the occasional extra or lack of count shouldn't make a significant
> difference.

I agree, but for another reason. I think it won't make a significant 
difference because rewrites coinciding with writebacks coinciding with 
truncations will happen very rare in real life.

Thanks,
Maxim


> Thanks,
> Miklos
>
> > Signed-off-by: Maxim Patlasov <MPatlasov@xxxxxxxxxxxxx>
> > ---
> >   fs/fuse/file.c |    6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> > index a3c7123..5d323bd 100644
> > --- a/fs/fuse/file.c
> > +++ b/fs/fuse/file.c
> > @@ -1536,6 +1536,7 @@ static void fuse_writepage_end(struct fuse_conn *fc, struct fuse_req *req)
> >   			drop->inode->i_mapping->backing_dev_info;
> >   		dec_bdi_stat(bdi, BDI_WRITEBACK);
> >   		dec_zone_page_state(drop->pages[0], NR_WRITEBACK_TEMP);
> > +		bdi_writeout_inc(bdi);
> >   		fuse_writepage_free(fc, drop);
> >   		fuse_put_request(fc, drop);
> >   		drop = next;
> > @@ -1706,11 +1707,14 @@ static bool fuse_writepage_in_flight(struct fuse_req *new_req,
> >   
> >   	if (old_req->num_pages == 1 && (old_req->state == FUSE_REQ_INIT ||
> >   					old_req->state == FUSE_REQ_PENDING)) {
> > +		struct backing_dev_info *bdi = page->mapping->backing_dev_info;
> > +
> >   		copy_highpage(old_req->pages[0], page);
> >   		spin_unlock(&fc->lock);
> >   
> > -		dec_bdi_stat(page->mapping->backing_dev_info, BDI_WRITEBACK);
> > +		dec_bdi_stat(bdi, BDI_WRITEBACK);
> >   		dec_zone_page_state(page, NR_WRITEBACK_TEMP);
> > +		bdi_writeout_inc(bdi);
> >   		fuse_writepage_free(fc, new_req);
> >   		fuse_request_free(new_req);
> >   		goto out;

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/