lkml.org 
[lkml]   [2007]   [May]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH 3/3] AFS: Implement basic file write support
Date
Andrew Morton <akpm@linux-foundation.org> wrote:

> > + BUG_ON(i_size > 0xffffffff); // TODO: use 64-bit store
>
> You're sure this isn't user-triggerable?

Hmmm... I'm not. I'll whip up a patch for this.

> kmap_atomic() could be used here and is better.

Yeah. It used to have something that slept in the middle of it, but that's no
longer there. I'll add to the patch.

> We have this zero_user_page() thing heading in which could perhaps be used
> here also.

Okay. I'll have a look at it once it's there.

> > + ASSERTRANGE(wb->first, <=, index, <=, wb->last);
>
> wow.

:-)

The assertions I've put in have been very useful.

> > + set_page_dirty(page);
> > +
> > + if (PageDirty(page))
> > + _debug("dirtied");
> > +
> > + return 0;
> > +}
>
> One would normally run mark_inode_dirty() after any i_size_write()?

Not in this case, I assume, because set_page_dirty() ultimately calls
__mark_inode_dirty(), but I could be wrong.

> We can invalidate pages and we can truncate them and we can clean them.
> But here we have a new operation, "killing". I wonder what that is.

I can call it invalidation if you like, though that name is already reserved
as it were:-/ I suppose it might actually make sense for me to call
invalidatepage() myself.

> > + if (wbc->sync_mode != WB_SYNC_NONE)
> > + wait_on_page_writeback(page);
>
> Didn't the VFS already do that?

I'm not entirely sure. Looking at generic_writepages(), I guess so. I'll
patch it out.

> > + if (PageWriteback(page) || !PageDirty(page)) {
> > + unlock_page(page);
> > + return 0;
> > + }
>
> And some of that?

Yeah. Seems so. I'll patch that out too.

What I'd like to do is ditch writepage() entirely - I'm not sure it's entirely
necessary with the availability of writepages() - but I'll look at that
another time.

> I have this vague prehistoric memory that something can go wrong at the VFS
> level if the address_space writes back more pages than it was asked to.
> But I forget what the issue was and it would be silly to have an issue
> with that anyway. Something to keep an eye out for.

Okay.

Thanks for the 'cherry-pick'. I'll hopefully have a revision patch for you
soon.

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

\
 
 \ /
  Last update: 2007-05-09 12:29    [W:0.066 / U:0.816 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site