All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Liu <jeff.liu@oracle.com>
To: linux-ext4@vger.kernel.org
Cc: Yongqiang Yang <xiaoqiangnk@gmail.com>,
	Sunil Mushran <sunil.mushran@oracle.com>
Subject: Add SEEK_DATA/SEEK_HOLE support V2
Date: Wed, 28 Sep 2011 14:32:27 +0800	[thread overview]
Message-ID: <4E82BF7B.7000403@oracle.com> (raw)

Hello all,

This is the V2 for add SEEK_DATA/SEEK_HOLE support to ext4.
In V1, I copied all the codes regarding fetching delayed-extents out to
my seek callback function, it's stupid to duplicate the code blocks.

Now the idea is, since the delayed-extents is the main concern to search
extents for both fiemap and seek, so IMHO, it's better to consolidate
this procedure into a new function(get_delayed_extent() in my patch),
and call it in fiemap and seek call-back functions if necessary.  In
this way, when yongqiang's delayed-extents tree is ready, we only need
to modify the code block at get_delayed_extent() accordingly.

In addition, yongqiang has pointed out there might be a deadlock, but
per my understood, as the target inode already locked via i_mutex in
ext4_llseek() before, this lock can prevent any modification to the
file, and we call find_get_pages_tag() when trying to fetch the dirty
pages, this function using rcu read lock, it also safe to read.  So I
have no idea what's wrong, could anyone please kindly point me out in
this case?  I'm definitely a newbie to this list, sorry for my ignorant.

Anyway, I'd like to post the revised for your guys review first.
In have split the modification to two patches this time, since I hope to
make it looks more convenient for the review purpose.

The 1st patch, remove the delayed-extents fetching procedure to a
function named as get_delayed_extent(), and modify the
ext4_ext_fiemap_cb() accordingly.

The 2nd path add the ext4_ext_seek_cb() to implement the
SEEK_HOLE/SEEK_DATA stuff based on previous code change.

Testing:
========
In addition to the test cases mentioned in V1, I also done another tests
through sunil's seek test program, please refer to:
http://oss.oracle.com/~smushran/seek_data/seek_test.c

To ensure the 1st patch does not cause regression issue on fiemap, I
have done some tests via cp(1) with fiemap copy support, the sparse file
used for this test is produced by:
python -c "f=open('./sptest', 'w'); [(f.seek(x) or f.write(str(x))) for
x in range(1, 1000000000, 99999)]; f.close()


Any feedback are appreciated!


Thanks,
-Jeff

             reply	other threads:[~2011-09-28  6:32 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-28  6:32 Jeff Liu [this message]
2011-09-28  7:21 ` Add SEEK_DATA/SEEK_HOLE support V2 Yongqiang Yang
2011-09-28  8:02   ` Jeff Liu
2011-09-28  8:11     ` Yongqiang Yang
2011-09-28  8:40       ` Jeff Liu
2011-09-28  8:53         ` Yongqiang Yang
2011-09-28  9:06           ` Jeff Liu

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=4E82BF7B.7000403@oracle.com \
    --to=jeff.liu@oracle.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=sunil.mushran@oracle.com \
    --cc=xiaoqiangnk@gmail.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.