[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20130626133913.GD3875@thunk.org>
Date: Wed, 26 Jun 2013 09:39:13 -0400
From: Theodore Ts'o <tytso@....edu>
To: Lukáš Czerner <lczerner@...hat.com>
Cc: Zheng Liu <gnehzuil.liu@...il.com>, linux-ext4@...r.kernel.org
Subject: Re: [PATCH FOR DISCUSSION] add delalloc debugging
On Wed, Jun 26, 2013 at 02:31:14PM +0200, Lukáš Czerner wrote:
> > 1. Now when we read /proc/fs/ext4/{$DEV}/delalloc_debug, it will print
> > the result in console. IMHO, I don't think it is a good choice. I
> > prefer to print this result in debugfs or in sysfs.
>
> I would prefer to print this info in debug_delalloc_show() so we can
> simply read the file instead of read the file + read the log.
This turns out to be tricky because we would need to grab all of this
information and stash it in kernel allocated memory, since otherwise
it might change out from under us if we need to read the information
across multiple read requests to the /proc file.
> I think that having this enabled only is EXT4_DEBUG is set is just
> fine. However if you want to be able to use it even without EXT4_DEBUG
> option we could just set proper permissions on the
> /proc/fs/ext4/{$DEV}/delalloc_debug so that only privileged user can
> open it.
The file is mode 400 readable by root already.
> > 3. Maybe we can provide a interface to let the user indicate which inode
> > they want to observe.
>
> you can grep for it, I do not think it's a big deal.
If we wanted to release the delalloc information for an inode, that's
actually easier; we could do this via an ioctl.
> > Finally, the patch itself still has two minor problems. We forget
> > to call remove_proc_entry() in ext4_put_super(). Another problem is
> > compile warnings.
This were alrady fixed in the version which is in the ext4 patch
queue. I noticed those issues shortly after I sent out the patch for
discussion.
- Ted
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists