From: Nick Piggin <npiggin@suse.de>
To: Boaz Harrosh <bharrosh@panasas.com>
Cc: Christoph Hellwig <hch@lst.de>,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
open-osd <osd-dev@open-osd.org>
Subject: Re: [RFC] exofs: New truncate sequence
Date: Tue, 1 Jun 2010 01:09:57 +1000 [thread overview]
Message-ID: <20100531150957.GM9453@laptop> (raw)
In-Reply-To: <4C03CC99.7030404@panasas.com>
On Mon, May 31, 2010 at 05:50:01PM +0300, Boaz Harrosh wrote:
> On 05/31/2010 05:33 PM, Nick Piggin wrote:
> > On Mon, May 31, 2010 at 05:13:34PM +0300, Boaz Harrosh wrote:
> >> On 05/31/2010 04:44 PM, Nick Piggin wrote:
> >>> On Mon, May 31, 2010 at 03:30:02PM +0300, Boaz Harrosh wrote:
> >>>> ---
> >>>> fs/exofs/exofs.h | 1 -
> >>>> fs/exofs/file.c | 1 -
> >>>> fs/exofs/inode.c | 115 +++++++++++++++++++++++++++---------------------------
> >>>
> >>> Can you rip out all the rest of the buffer_head stuff too?
> >>>
> >>
> >> I hope I don't have any left, that was the last, have I missed
> >> something?
> >
> > exofs_invalidatepage, exofs_releasepage, includes of buffer_head.h.
> > No point to any of that if you never actually map the buffers or
> > use them for tracking state yourself.
> >
>
> Rrr thanks. Yes I'll leave the WARN_ON and do nothing, and remove the
> include. I'll submit for linux-next.
OK. I'd get rid of it completely before it hits mainline.
> Thanks.
>
> <snip>
>
> >
> >> You see this is where exofs is different a file is an object_no on
> >> multiple OSD devices. The inode is kept as an attribute of the
> >> object. (data as object's data) so a exofs_sbi_remove will just
> >> obliterate any association to the object. It was historically
> >> called because exofs_truncate used to do what truncate_inode_pages
> >> does today. (And some other in memory book keeping.) But with
> >> your help all this was cleaned up.
> >
> > OK, I was thinking the underlying object itself needs to be trimmed
> > to match i_size similarly to just a block based filesystem? Like
> > exofs_oi_truncate appears to.
> >
>
> Yes you are right, generally, but since I'm doing exofs_sbi_remove()
> just after that then there is no point. the osd_remove will take care of
> that anyway.
I fugured it would, just wanted to check.
> >> Do you see any operation I missed that might need cleaning from the
> >> generic VFS inode, that might now leak. As far as storage is concerned
> >> I'm covered.
> >>
> >> [I ran git clone linux; rm -rf linux; 100 times in a loop and the OSD
> >> storage stayed constant size. So I presume there is no storage leak.
> >> OSD is good in this respect]
> >
> > I can't see anything off hand. Was just flagging points where
> > vmtruncate or truncate had been called and is not now. If you
> > have all those covered, then you should be OK.
> >
> >
>
> I'll be testing this particular branch for a while. I have a chicken
> and egg problem. I think I'm kind of dependent on Al's vfs for-next
> branch. Do you think the patch could also work with Linus-2.6.35-rc1?
> I'm afraid that even if so It might conflict with Al's tree if I put
> it independently in the osd/for-next tree.
I would keep them based on top of Christoph's patches (just cherry
pick the exofs hunks in the meantime).
next prev parent reply other threads:[~2010-05-31 15:10 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-31 12:30 [RFC] exofs: New truncate sequence Boaz Harrosh
2010-05-31 13:44 ` Nick Piggin
2010-05-31 14:13 ` Boaz Harrosh
2010-05-31 14:33 ` Nick Piggin
2010-05-31 14:50 ` Boaz Harrosh
2010-05-31 15:09 ` Nick Piggin [this message]
2010-05-31 15:19 ` Boaz Harrosh
2010-06-01 10:08 ` Christoph Hellwig
2010-06-01 10:26 ` Boaz Harrosh
2010-06-01 10:44 ` Christoph Hellwig
2010-06-01 11:05 ` Boaz Harrosh
2010-06-01 11:06 ` Christoph Hellwig
2010-06-01 10:28 ` [PATCH ver2] " Boaz Harrosh
2010-06-01 10:43 ` Christoph Hellwig
2010-06-01 10:59 ` Boaz Harrosh
2010-06-01 11:06 ` Christoph Hellwig
2010-06-01 11:31 ` [PATCH ver3] " Boaz Harrosh
2010-06-01 11:36 ` Christoph Hellwig
2010-06-01 11:52 ` Boaz Harrosh
2010-06-01 15:09 ` Nick Piggin
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=20100531150957.GM9453@laptop \
--to=npiggin@suse.de \
--cc=bharrosh@panasas.com \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=osd-dev@open-osd.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).