lkml.org 
[lkml]   [2009]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 3/8] writeback: switch to per-bdi threads for flushing data v2
On Mon, Sep 07 2009, Jan Kara wrote:
> Hi Jens,
>
> now I've found just two minor things (see below). Besides them the only
> thing which is blocking my ack is a way to effectively lookup a BDI from a
> superblock so that we can reasonably effectively fsync a superblock...

Can we side step that as an inclusion criteria, please? I'd really like
to work on adding that quick lookup, but I'd also hate to potentiall
destabilize anything at this point. So we can probably quite easily make
.32 as well for that, but I'd rather not risk doing a version and
include that from the beginning. OK?

> > + /*
> > + * Check for periodic writeback, kupdated() style
> > + */
> > + if (!wrote)
> > + wrote = wb_check_old_data_flush(wb);
> Why is here the !wrote check? It would feel safer if we just did
> wrote += wb_check_old_data_flush(wb);
> Otherwise we cannot guarantee syncing of inodes every writeback_interval.

Yes good point, that check should not be there or the logic is still
broken wrt old data flushing. Will fix that, thanks!

> > + /*
> > + * If work allocation fails, do the writes inline. We drop
> > + * the lock and restart the list writeout. This should be OK,
> > + * since this happens rarely and because the writeout should
> > + * eventually make more free memory available.
> > + */
> > + work = bdi_alloc_work(wbc);
> > + if (!work) {
> > + struct writeback_control __wbc = *wbc;
> >
> > - cond_resched();
> > + /*
> > + * Not a data integrity writeout, just continue
> > + */
> > + if (!must_wait)
> > + continue;
> >
> > - spin_lock(&inode_lock);
> > + spin_unlock(&bdi_lock);
> > + __wbc = *wbc;
> You initialize the variable twice...

Indeed, the latter should just go. Good spotting, fixed.

--
Jens Axboe



\
 
 \ /
  Last update: 2009-09-07 20:47    [W:0.159 / U:0.008 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site