[Ocfs2-devel] [patch 09/11] ocfs2: llseek requires ocfs2 inode lock for the file in SEEK_END

[Jensen]

On 2014/2/8 10:07, Mark Fasheh wrote:
> On Sat, Feb 08, 2014 at 09:26:03AM +0800, Jensen wrote:
>> On 2014/2/8 6:44, Mark Fasheh wrote:
>>> On Thu, Feb 06, 2014 at 03:50:29PM -0800, Andrew Morton wrote:
>>>> On Thu, 6 Feb 2014 15:42:53 -0800 Mark Fasheh <mfasheh at suse.de> wrote:
>>>>
>>>>> On Fri, Jan 24, 2014 at 12:47:09PM -0800, akpm at linux-foundation.org wrote:
>>>>>> From: Jensen <shencanquan at huawei.com>
>>>>>> Subject: ocfs2: llseek requires ocfs2 inode lock for the file in SEEK_END
>>>>>>
>>>>>> llseek requires ocfs2 inode lock for updating the file size in SEEK_END. 
>>>>>> because the file size maybe update on another node.
>>>>>>
>>>>>> This bug can be reproduce the following scenario: at first, we dd a test
>>>>>> fileA, the file size is 10k.
>>>>> Basically, you want to amke SEEK_END cluster-aware. This patch would be the
>>>>> right way to do it.
>>>> Sunil was worried about the performance impact.  Correctness beats
>>>> performance, but some quantitative testing would be useful?
>>> Performance is my primary concern as well. I thought of writing it up but
>>> realized I don't really have any evidence off the top of my head one way or
>>> the other that this might slow us down.
>>>
>>> That said, I kind of question the usefulness of this patch - we got
>>> along pretty well so far without locking in lseek and some random dd(1) test
>>> doesn't really provide a great end-user reason for why we should do this.
>>>
>>> I will note that gfs2 locks for SEEK_END.
>>>
>>>
>>> Testing would help to answer this, yes. Jensen is this something you can do?
>>> I'm not sure exactly what we would run yet though, I have to think about
>>> that (or maybe someone can suggest something).
>>>
>>> Thanks,
>>> 	--Mark
>>>
>>    ocfs2 is a cluster file system.  as like read/write/open/rmdir/unlink interface which think of cluster-aware. I think the seek interface need
>>    cluster-aware. May be it has the performance impact. but it's correctness is more important than performance.
> That doesn't mean we can't or shouldn't quantify the performance impact of your patch.
>
> Please help us measure what the end-user impact of this change will be.
> 	--Mark
  test result: 1000000 times lseek call;
  index        lseek with inode lock (unit:us)                lseek without inode lock (unit:us)
    1                   1168162                                    555383
    2                   1168011                                    549504
    3                   1170538                                    549396
    4                   1170375                                    551685
    5                   1170444                                    556719
    6                   1174364                                    555307
    7                   1163294                                    551552
    8                   1170080                                    549350
    9                   1162464                                    553700
  10                   1165441                                    552594

 avg                  1168317                                    552519

avg with lock - avg without lock = 615798
(avg with lock - avg without lock)/1000000=0.615798 us

>
> --
> Mark Fasheh
>
> .
>