Daniel Phillips wrote:
> Alexander Viro wrote:
> > On Tue, 19 Sep 2000, Daniel Phillips wrote:
> >
> > > !Mapped, !Uptodate, Dirty: pending map and write
> >
> > Wrong. It's an instant BUG at line 711 in ll_rw_blk.c - remember these
> > reports?
Yes, correct, this state should not escape
Alexander Viro wrote:
>
> On Tue, 19 Sep 2000, Daniel Phillips wrote:
>
> > The more I think about it the less clear and ambiguous I find it.
> > When you add the dirty bit into the pot you get:
> >
> >Mapped, Uptodate, Dirty: not possible
>
> Sure, it is possible - that's how the
On Tue, 19 Sep 2000, Daniel Phillips wrote:
> The more I think about it the less clear and ambiguous I find it.
> When you add the dirty bit into the pot you get:
>
>Mapped, Uptodate, Dirty: not possible
Sure, it is possible - that's how the write happens
> !Mapped,
Linus Torvalds wrote:
>
> On Mon, 18 Sep 2000, Alexander Viro wrote:
> > * we have several bh state components and the thing is a big,
> > fscking mess. If we look at the areas outside of the page lock we have:
>
> It's not a mess at all.
>
> I don't see why you call things "first stage"
On Tue, 19 Sep 2000, Rik van Riel wrote:
> IMHO it would be really nice if this problem was solved
> on the /PAGE/ level, so it will work for non-buffer
> filesystems as well ;)
It would be nice if we separated the buffer-cache storage pages from the
pagecache buffer-using ones and from the
On Mon, 18 Sep 2000, Alexander Viro wrote:
> On Sun, 17 Sep 2000, Linus Torvalds wrote:
>
> > And as we've seen, simplifying this area would not necessarily be a bad
> > thing ;)
> >
> > Right now I'll just do the minimal fix, though, I think.
>
> Fine with me. I'll do the full analysis
On Mon, 18 Sep 2000, Alexander Viro wrote:
On Sun, 17 Sep 2000, Linus Torvalds wrote:
And as we've seen, simplifying this area would not necessarily be a bad
thing ;)
Right now I'll just do the minimal fix, though, I think.
Fine with me. I'll do the full analysis tonight, anyway,
Linus Torvalds wrote:
On Mon, 18 Sep 2000, Alexander Viro wrote:
* we have several bh state components and the thing is a big,
fscking mess. If we look at the areas outside of the page lock we have:
It's not a mess at all.
I don't see why you call things "first stage" etc. It's
On Tue, 19 Sep 2000, Daniel Phillips wrote:
The more I think about it the less clear and ambiguous I find it.
When you add the dirty bit into the pot you get:
Mapped, Uptodate, Dirty: not possible
Sure, it is possible - that's how the write happens
!Mapped, Uptodate,
Alexander Viro wrote:
On Tue, 19 Sep 2000, Daniel Phillips wrote:
The more I think about it the less clear and ambiguous I find it.
When you add the dirty bit into the pot you get:
Mapped, Uptodate, Dirty: not possible
Sure, it is possible - that's how the write happens
Daniel Phillips wrote:
Alexander Viro wrote:
On Tue, 19 Sep 2000, Daniel Phillips wrote:
!Mapped, !Uptodate, Dirty: pending map and write
Wrong. It's an instant BUG at line 711 in ll_rw_blk.c - remember these
reports?
Yes, correct, this state should not escape from
Andreas Dilger wrote:
>
> Daniel writes:
> > Alexander Viro wrote:
> > > On Mon, 18 Sep 2000, Andreas Dilger wrote:
> > > > This may actually be a problem in the future... what about shared access
> > > > block devices like FCAL or a distributed filesystem? It has to be
> > > > possible for
Linus Torvalds wrote:
>
> On Mon, 18 Sep 2000, Alexander Viro wrote:
> > That's what makes me unhappy about the current situation + obvious
> > fixes. It works, but the proof is... well, not pretty.
>
> Oh, agreed. I think we should clean up the code. I looked at it yesterday,
> and it
Daniel writes:
> Alexander Viro wrote:
> > On Mon, 18 Sep 2000, Andreas Dilger wrote:
> > > This may actually be a problem in the future... what about shared access
> > > block devices like FCAL or a distributed filesystem? It has to be
> > > possible for pages to become non-uptodate in a sane
Alexander Viro wrote:
>
> On Mon, 18 Sep 2000, Andreas Dilger wrote:
>
> > Alexander writes:
> > > * uptodate pages should never become non-uptodate.
> > > uptodate .. pages ... never have data _older_ than on disk
> >
> > This may actually be a problem in the future... what about
On Mon, 18 Sep 2000, Alexander Viro wrote:
> >
> > but should instead be
> >
> > static int make_buffer_uptodate(struct page *page, struct buffer_head * bh)
> > {
> > if (Page_Uptodate(page)) {
> ^^^
> >
That's to fs/buffer.c, generic_file_write() part will go
separately (it's a separate bug actually - if copy_from_user() in
generic_file_write() fails we mark page as not-uptodate; if memory
pressure will drop the buffer_heads attempts to read will blow the page
contents away).
On Mon, 18 Sep 2000, Alexander Viro wrote:
> Except that the only reason why check for page being uptodate is correct
> is that for such pages we are guaranteed that buffer ring will not be
> dropped when page contatins data newer than on disk.
Ugh. Translation:
The only reason why the
On Mon, 18 Sep 2000, Linus Torvalds wrote:
> It's not a mess at all.
>
> I don't see why you call things "first stage" etc. It's perfectly
> straightforward. There are two bits that matter:
> - buffer uptodate
> - buffer mapped.
first stage == page still not uptodate. second stage ==
On Mon, 18 Sep 2000, Alexander Viro wrote:
> * we have several bh state components and the thing is a big,
> fscking mess. If we look at the areas outside of the page lock we have:
It's not a mess at all.
I don't see why you call things "first stage" etc. It's perfectly
straightforward.
--On 09/18/00 13:19:27 -0400 Alexander Viro <[EMAIL PROTECTED]> wrote:
>
>
> On Mon, 18 Sep 2000, Chris Mason wrote:
>
>> I'm not trying to put it all into a single get_block call, we have
>> different get_block funcs for different purposes. What I'm really trying
>> to do is squeeze into
OK, let's see. I've tried to describe what we have now (marking the
bugs) + proposal that would give somewhat saner logics (in the end of
posting). Comments are more than welcome.
* life of the page is clearly divided in two parts - before it can
be mapped to a user context and after
On Mon, 18 Sep 2000, Chris Mason wrote:
> I'm not trying to put it all into a single get_block call, we have
> different get_block funcs for different purposes. What I'm really trying
> to do is squeeze into block_prepare_write, as a generic setup function for
> file modifications.
It is
--On 09/18/00 12:23:43 -0400 Alexander Viro <[EMAIL PROTECTED]> wrote:
> On Mon, 18 Sep 2000, Chris Mason wrote:
>
>> When reiserfs_get_block is called on the packed data (with create == 1),
>> we unpack it to a full block, so the generic functions can handle
>> dirties/writes from then on. If
--On 09/18/00 08:52:12 -0700 Linus Torvalds <[EMAIL PROTECTED]> wrote:
>
>
> On Mon, 18 Sep 2000, Chris Mason wrote:
>>
>> ReiserFS depends on the buffer head up to date flag being correct when it
>> is sent to get_block. When unpacking the tail, we have to know if the
>> packed data on
On Mon, 18 Sep 2000, Chris Mason wrote:
>
> ReiserFS depends on the buffer head up to date flag being correct when it
> is sent to get_block. When unpacking the tail, we have to know if the
> packed data on disk should be copied over the data in the page.
>
> So, the above should work for
Alexander Viro wrote:
>
> On Sun, 17 Sep 2000, Linus Torvalds wrote:
>
> > On Sun, 17 Sep 2000, Alexander Viro wrote:
> > >
> > > Looks sane. But I really wonder if we could just do it in
> > > create_page_buffers() if page is up-to-date. OTOH it would require attempt
> > > to map them all.
--On 09/18/00 09:16:22 -0400 Alexander Viro <[EMAIL PROTECTED]> wrote:
>
>
> On Mon, 18 Sep 2000, Chris Mason wrote:
>
>> ReiserFS depends on the buffer head up to date flag being correct when it
>> is sent to get_block. When unpacking the tail, we have to know if the
>> packed data on
On Mon, 18 Sep 2000, Chris Mason wrote:
> ReiserFS depends on the buffer head up to date flag being correct when it
> is sent to get_block. When unpacking the tail, we have to know if the
> packed data on disk should be copied over the data in the page.
??? Details, please. What the hell
--On 09/17/00 20:30:29 -0700 Linus Torvalds <[EMAIL PROTECTED]> wrote:
>
> Basically, both "truncate()" and "write()" have this bug where they can
> end up re-reading stuff from disk even though the in-memory copy is newer.
>
> And because write() had this bug, the bug also got into
>
--On 09/17/00 20:30:29 -0700 Linus Torvalds [EMAIL PROTECTED] wrote:
Basically, both "truncate()" and "write()" have this bug where they can
end up re-reading stuff from disk even though the in-memory copy is newer.
And because write() had this bug, the bug also got into
On Mon, 18 Sep 2000, Chris Mason wrote:
ReiserFS depends on the buffer head up to date flag being correct when it
is sent to get_block. When unpacking the tail, we have to know if the
packed data on disk should be copied over the data in the page.
??? Details, please. What the hell are
Alexander Viro wrote:
On Sun, 17 Sep 2000, Linus Torvalds wrote:
On Sun, 17 Sep 2000, Alexander Viro wrote:
Looks sane. But I really wonder if we could just do it in
create_page_buffers() if page is up-to-date. OTOH it would require attempt
to map them all. Comments?
On Mon, 18 Sep 2000, Chris Mason wrote:
I'm not trying to put it all into a single get_block call, we have
different get_block funcs for different purposes. What I'm really trying
to do is squeeze into block_prepare_write, as a generic setup function for
file modifications.
It is not.
--On 09/18/00 13:19:27 -0400 Alexander Viro [EMAIL PROTECTED] wrote:
On Mon, 18 Sep 2000, Chris Mason wrote:
I'm not trying to put it all into a single get_block call, we have
different get_block funcs for different purposes. What I'm really trying
to do is squeeze into
Alexander Viro wrote:
On Mon, 18 Sep 2000, Andreas Dilger wrote:
Alexander writes:
* uptodate pages should never become non-uptodate.
uptodate .. pages ... never have data _older_ than on disk
This may actually be a problem in the future... what about shared access
Daniel writes:
Alexander Viro wrote:
On Mon, 18 Sep 2000, Andreas Dilger wrote:
This may actually be a problem in the future... what about shared access
block devices like FCAL or a distributed filesystem? It has to be
possible for pages to become non-uptodate in a sane way.
So
Linus Torvalds wrote:
On Mon, 18 Sep 2000, Alexander Viro wrote:
That's what makes me unhappy about the current situation + obvious
fixes. It works, but the proof is... well, not pretty.
Oh, agreed. I think we should clean up the code. I looked at it yesterday,
and it didn't look
Andreas Dilger wrote:
Daniel writes:
Alexander Viro wrote:
On Mon, 18 Sep 2000, Andreas Dilger wrote:
This may actually be a problem in the future... what about shared access
block devices like FCAL or a distributed filesystem? It has to be
possible for pages to become
That's to fs/buffer.c, generic_file_write() part will go
separately (it's a separate bug actually - if copy_from_user() in
generic_file_write() fails we mark page as not-uptodate; if memory
pressure will drop the buffer_heads attempts to read will blow the page
contents away).
On Mon, 18 Sep 2000, Alexander Viro wrote:
but should instead be
static int make_buffer_uptodate(struct page *page, struct buffer_head * bh)
{
if (Page_Uptodate(page)) {
^^^
set_bit(BH_Uptodate,
On Mon, 18 Sep 2000, Alexander Viro wrote:
* we have several bh state components and the thing is a big,
fscking mess. If we look at the areas outside of the page lock we have:
It's not a mess at all.
I don't see why you call things "first stage" etc. It's perfectly
straightforward.
On Mon, 18 Sep 2000, Linus Torvalds wrote:
It's not a mess at all.
I don't see why you call things "first stage" etc. It's perfectly
straightforward. There are two bits that matter:
- buffer uptodate
- buffer mapped.
first stage == page still not uptodate. second stage == page is
On Mon, 18 Sep 2000, Alexander Viro wrote:
Except that the only reason why check for page being uptodate is correct
is that for such pages we are guaranteed that buffer ring will not be
dropped when page contatins data newer than on disk.
Ugh. Translation:
The only reason why the simple
On Sun, 17 Sep 2000, Linus Torvalds wrote:
> At that point, block_write_full_page() would become something like
>
> unsigned long index = inode->i_size >> PAGE_CACHE_SHIFT;
>
> offset = inode->i_size & (PAGE_CACHE_SIZE-1);
> if (index > page->index)
> offset =
On Sun, 17 Sep 2000, Linus Torvalds wrote:
> On Sun, 17 Sep 2000, Alexander Viro wrote:
> >
> > Looks sane. But I really wonder if we could just do it in
> > create_page_buffers() if page is up-to-date. OTOH it would require attempt
> > to map them all. Comments?
>
> That would certainly
On Mon, 18 Sep 2000, Alexander Viro wrote:
>
> We might as well do it in create_empty_buffers(), FWIW. Do you see any
> case when it would mean extra work? I'm thinking about the following:
>
> if (page is uptodate)
> do get_block(...,0) for all buffers and mark them
On Sun, 17 Sep 2000, Alexander Viro wrote:
>
> Looks sane. But I really wonder if we could just do it in
> create_page_buffers() if page is up-to-date. OTOH it would require attempt
> to map them all. Comments?
That would certainly simplify a lot.
And as we've seen, simplifying this
On Sun, 17 Sep 2000, Linus Torvalds wrote:
> Yeah. See my other mail - I don't think the patch is needed at all,
> although I'm convinced that it would actually hide the problem for the
> particular case of INN (zeroing out the partial block is not an issue
> there, as truncate will have done
On Sun, 17 Sep 2000, Linus Torvalds wrote:
> The basic problem is that right now we have code that does
>
> if (!page->buffers)
> create_empty_buffers(page, inode, inode->i_sb->s_blocksize);
> ...
> if (!buffer_uptodate(bh))
> ll_rw_block(READ,
On Sun, 17 Sep 2000, Alexander Viro wrote:
>
> Ugh. OK, first of all, patch is _not_ correct. Doesn't zero out the end of
> partial block.
>
Yeah. See my other mail - I don't think the patch is needed at all,
although I'm convinced that it would actually hide the problem for the
particular
On Sun, 17 Sep 2000, Linus Torvalds wrote:
>
> I bet your patch fixes the corruption, but I want to understand _why_.
> Call me dense, but __block_commit_write() seems to do everything we want
> done..
Ok, I may be dense, but I see the bug.
And no, your patch was not the right thing either.
On Sun, 17 Sep 2000, Linus Torvalds wrote:
> Hmm.. Ok, I see what you're saying. And I looked at the diff the wrong way
> around - we _lost_ two articles rather than getting two new ones.
>
> However, what's wrong with the block_write_full_page() logic? Your patch
> looks fine to me, but so
On Sun, 17 Sep 2000, Linus Torvalds wrote:
>
>
> On Sun, 17 Sep 2000, Marco d'Itri wrote:
> >
> > I added to block_write_full_page() the debugging code suggested by
> > Alexander Viro:
> >
> > if (inode->i_dev == 0x305 && inode->i_ino == 48991) // Md
> >
I added to block_write_full_page() the debugging code suggested by
Alexander Viro:
if (inode->i_dev == 0x305 && inode->i_ino == 48991) // Md
printk("block_write_full_page: writing page %d, size %Ld\n",
page->index, inode->i_size);
and I have
On Sun, 17 Sep 2000, Marco d'Itri wrote:
>
> I added to block_write_full_page() the debugging code suggested by
> Alexander Viro:
>
> if (inode->i_dev == 0x305 && inode->i_ino == 48991) // Md
> printk("block_write_full_page: writing page %d, size %Ld\n",
>
On Sun, 17 Sep 2000, Marco d'Itri wrote:
I added to block_write_full_page() the debugging code suggested by
Alexander Viro:
if (inode-i_dev == 0x305 inode-i_ino == 48991) // Md
printk("block_write_full_page: writing page %d, size %Ld\n",
I added to block_write_full_page() the debugging code suggested by
Alexander Viro:
if (inode-i_dev == 0x305 inode-i_ino == 48991) // Md
printk("block_write_full_page: writing page %d, size %Ld\n",
page-index, inode-i_size);
and I have just
On Sun, 17 Sep 2000, Linus Torvalds wrote:
On Sun, 17 Sep 2000, Marco d'Itri wrote:
I added to block_write_full_page() the debugging code suggested by
Alexander Viro:
if (inode-i_dev == 0x305 inode-i_ino == 48991) // Md
On Sun, 17 Sep 2000, Linus Torvalds wrote:
Hmm.. Ok, I see what you're saying. And I looked at the diff the wrong way
around - we _lost_ two articles rather than getting two new ones.
However, what's wrong with the block_write_full_page() logic? Your patch
looks fine to me, but so does
On Sun, 17 Sep 2000, Linus Torvalds wrote:
I bet your patch fixes the corruption, but I want to understand _why_.
Call me dense, but __block_commit_write() seems to do everything we want
done..
Ok, I may be dense, but I see the bug.
And no, your patch was not the right thing either. It
On Sun, 17 Sep 2000, Alexander Viro wrote:
Ugh. OK, first of all, patch is _not_ correct. Doesn't zero out the end of
partial block.
note to myself: no patches before the first cup of coffee
Yeah. See my other mail - I don't think the patch is needed at all,
although I'm convinced that it
On Sun, 17 Sep 2000, Linus Torvalds wrote:
The basic problem is that right now we have code that does
if (!page-buffers)
create_empty_buffers(page, inode, inode-i_sb-s_blocksize);
...
if (!buffer_uptodate(bh))
ll_rw_block(READ, 1, bh);
On Sun, 17 Sep 2000, Linus Torvalds wrote:
Yeah. See my other mail - I don't think the patch is needed at all,
although I'm convinced that it would actually hide the problem for the
particular case of INN (zeroing out the partial block is not an issue
there, as truncate will have done it -
On Sun, 17 Sep 2000, Alexander Viro wrote:
Looks sane. But I really wonder if we could just do it in
create_page_buffers() if page is up-to-date. OTOH it would require attempt
to map them all. Comments?
That would certainly simplify a lot.
And as we've seen, simplifying this area
On Sun, 17 Sep 2000, Linus Torvalds wrote:
On Sun, 17 Sep 2000, Alexander Viro wrote:
Looks sane. But I really wonder if we could just do it in
create_page_buffers() if page is up-to-date. OTOH it would require attempt
to map them all. Comments?
That would certainly simplify a
On Mon, 18 Sep 2000, Alexander Viro wrote:
We might as well do it in create_empty_buffers(), FWIW. Do you see any
case when it would mean extra work? I'm thinking about the following:
if (page is uptodate)
do get_block(...,0) for all buffers and mark them uptodate
On Sun, 17 Sep 2000, Linus Torvalds wrote:
At that point, block_write_full_page() would become something like
unsigned long index = inode-i_size PAGE_CACHE_SHIFT;
offset = inode-i_size (PAGE_CACHE_SIZE-1);
if (index page-index)
offset =
68 matches
Mail list logo
