All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff Liu <jeff.liu@oracle.com>
To: Mark Tinguely <tinguely@sgi.com>
Cc: Christoph Hellwig <hch@infradead.org>, Ben Myers <bpm@sgi.com>,
	Chris Mason <chris.mason@oracle.com>,
	xfs@oss.sgi.com
Subject: Re: [PATCH] Introduce SEEK_DATA/SEEK_HOLE support to XFS V6
Date: Thu, 02 Feb 2012 11:38:05 +0800	[thread overview]
Message-ID: <4F2A051D.7050608@oracle.com> (raw)
In-Reply-To: <4F283F71.1020606@sgi.com>

On 02/01/2012 03:22 AM, Mark Tinguely wrote:

> On 01/29/12 02:35, Jeff Liu wrote:
>> Hello,
>>
>> Sorry for the delay!! I just got back from vacation.
>>
>> This is the V6 to introduce SEEK_DATA/SEEK_HOLE support to XFS.
>> As we have discussed previously, I have removed the dirty data probing
>> stuff and just treating unwritten extents as data in this post.
>>
>> Changes to V6:
>> --------------
>> * remove xfs_has_unwritten_buffer() for now.
>> * xfs_bmapi_read() returns the br_state == XFS_EXT_NORM for a hole, so
>> we need to check its startblock is not a "nullstartblock" in this case.
>> * call i_size_read() after taking the ilock shared, otherwise, isize
>> could be stale.
>> * remove "ifp = XFS_IFORK_PTR(ip, XFS_DATA_FORK)" from xfs_seek_data()
>> since it will not used.
>> * in xfs_file_llseek(), return -EINVAL rather than -EOPNOTSUPP if
>> whence is not valid according to http://linux.die.net/man/2/lseek.
>> * s/int lock/uint lock/ in both xfs_seek_data() and xfs_seek_hole().
>> * s/out_lock/out_unlock/ in both functions too.
>>
>> Tests:
>> ------
>> * seek_sanity_tester:
>> http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42514
>>
>> *seek_copy_tester:
>> http://permalink.gmane.org/gmane.comp.file-systems.xfs.general/42522
>>
>>
>> Thank you!
>> -Jeff
>>
>>
>> Signed-off-by: Jie Liu<jeff.liu@oracle.com>
>>
>> ---
>>   fs/xfs/xfs_file.c |  168
>> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>   1 files changed, 167 insertions(+), 1 deletions(-)
>>
>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
>> index 753ed9b..41a045f 100644
>> --- a/fs/xfs/xfs_file.c
>> +++ b/fs/xfs/xfs_file.c
>> +STATIC loff_t
>> +xfs_seek_data(
> <deleted>
>> +        error = xfs_bmapi_read(ip, fsbno, len - fsbno, map,&nmap,
>> +                       XFS_BMAPI_ENTIRE);
>> +        if (error)
>> +            goto out_unlock;
> 
> 
>> +STATIC loff_t
>> +xfs_seek_hole(
> <deleted>
>> +    error = xfs_bmap_first_unused(NULL, ip, 1,&fsbno, XFS_DATA_FORK);
>> +    if (error)
>> +        goto out_unlock;
> 
> The code looks good for the reduced problem. It test correctly. I am
> still finding holes only if they start on a 16KB boundary which we
> discussed before. I mention it in case more advanced test cases are
> generated.

> 
> Question:
> If the routines that are looking for extents/hole return an error (I see
> EFSCORRUPTED, EAGAIN, ENOMEM, EIO as possible errors in these routines),
> should you convert them to an error such as EIO? There is no specific
> error mention in the lseek manual page.

Thanks for your review!
The 'errno' should be convert to -ENXIO in those cases(this is the way I
observed from Btrfs and OCFS2), I'll take care of it in next post.


Thanks,
-Jeff

> 
> --Mark Tinguely.
> 
> _______________________________________________
> xfs mailing list
> xfs@oss.sgi.com
> http://oss.sgi.com/mailman/listinfo/xfs


_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

      reply	other threads:[~2012-02-02  3:38 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-01-29  8:35 [PATCH] Introduce SEEK_DATA/SEEK_HOLE support to XFS V6 Jeff Liu
2012-01-31 19:22 ` Mark Tinguely
2012-02-02  3:38   ` Jeff Liu [this message]

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=4F2A051D.7050608@oracle.com \
    --to=jeff.liu@oracle.com \
    --cc=bpm@sgi.com \
    --cc=chris.mason@oracle.com \
    --cc=hch@infradead.org \
    --cc=tinguely@sgi.com \
    --cc=xfs@oss.sgi.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.