From mboxrd@z Thu Jan 1 00:00:00 1970 From: Josef Bacik Subject: Re: Usefulness of SEEK_HOLE / SEEK_DATA in generic_file_llseek() Date: Mon, 23 Dec 2013 19:34:18 -0500 Message-ID: <52B8D68A.9050902@fb.com> References: <20131223231253.GA8376@quack.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit To: Jan Kara , Return-path: Received: from mx0a-00082601.pphosted.com ([67.231.145.42]:35090 "EHLO mx0a-00082601.pphosted.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758146Ab3LXAe2 (ORCPT ); Mon, 23 Dec 2013 19:34:28 -0500 In-Reply-To: <20131223231253.GA8376@quack.suse.cz> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On 12/23/2013 06:12 PM, Jan Kara wrote: > Hello, > > so I've now hit a xfstests failure for UDF which is caused by the > implementation of SEEK_HOLE / SEEK_DATA in generic_file_llseek(). UDF uses > that function as its .llseek method but it supports holes as any other unix > filesystem (e.g. ext2). The test in xfstests assumes that when it creates a > file by pwrite(fd, buf, bufsz, off), then SEEK_DATA on offset 0 should > return 'off' (off is reasonably rounded) but that's not true for the > implementation in generic_file_llseek(). > > Now I'm not so much interested in that test itself - that can be tweaked to > detect that case. But I rather wanted to ask - how useful is it to > implement SEEK_HOLE / SEEK_DATA the way it is in generic_file_llseek()? > Because it seems to me that any serious user will have to detect whether > SEEK_HOLE / SEEK_DATA works reasonably and if not, fall back to some > heuristic anyway. So why bother inventing bogus values in > generic_file_llseek and thus making detection of working implementation > harder? I'm writing this from my in-laws so I'm going to make some assumptions about how the code works based on my memory, so sorry in advanced if this is completely wrong ;). IIRC with the generic implementation we treat everything <= i_size as data and i_size as the first hole. The way the spec works is that if we are currently at data and do seek_data then we just return our current offset, same for a hole. In order to not be a jackass and have -EOPNOTSUPP for anybody who didn't implement seek_hole/seek_data I just did it this way where the only hole is the one that starts at i_size, so seek_data before that is going to return the value. As far as detecting an optimized handling of seek_hole/seek_data I'm not sure what the best answer for that is. I suppose seek_hole/seek_data is new enough that people will have checks for -EOPNOTSUPP anyway so we could just switch it back to that, but that seems like a regression of sorts to me. I'm not married to the implementation as it is so I'm open to suggestions. Thanks, Josef