[Ocfs2-devel] Question about commit cf1b5ea1c5cd

[Ocfs2-devel] Question about commit cf1b5ea1c5cd

[Joseph Qi]

Hi Viro,
I have questions about your commit cf1b5ea1c5cd
("[regression] ocfs2: do *not* increment ->ki_pos twice").
Yes, ->ki_pos is increased in generic_file_direct_write(). But
*ppos doesn't. So I increase it here for further use in
generic_perform_write.
After this, ->ki_pos and *ppos are equal. Then after buffer write, I
only get written_buffered increased into ->ki_pos, so ->ki_pos is *not*
increased twice.

[Ocfs2-devel] Question about commit cf1b5ea1c5cd

[Al Viro]

On Tue, Apr 14, 2015 at 11:06:40AM +0800, Joseph Qi wrote:
> Hi Viro,
> I have questions about your commit cf1b5ea1c5cd
> ("[regression] ocfs2: do *not* increment ->ki_pos twice").
> Yes, ->ki_pos is increased in generic_file_direct_write(). But
> *ppos doesn't. So I increase it here for further use in
> generic_perform_write.
> After this, ->ki_pos and *ppos are equal.

They are equal all along, for a very simple reason: ppos *points* *to*
iocb->ki_pos.  So yes, you do increase it twice, once via one alias,
once via another.

Check and you'll see - ppos is initialized with &iocb->ki_pos and never
reassigned.  What happens is an equivalent of

int x = 0;
int *p = &x;

x += 10;
*p += 10;

which obviously ends with x increased by 20, not by 10...

[Ocfs2-devel] Question about commit cf1b5ea1c5cd

[Joseph Qi]

On 2015/4/14 11:40, Al Viro wrote:
> On Tue, Apr 14, 2015 at 11:06:40AM +0800, Joseph Qi wrote:
>> Hi Viro,
>> I have questions about your commit cf1b5ea1c5cd
>> ("[regression] ocfs2: do *not* increment ->ki_pos twice").
>> Yes, ->ki_pos is increased in generic_file_direct_write(). But
>> *ppos doesn't. So I increase it here for further use in
>> generic_perform_write.
>> After this, ->ki_pos and *ppos are equal.
> 
> They are equal all along, for a very simple reason: ppos *points* *to*
> iocb->ki_pos.  So yes, you do increase it twice, once via one alias,
> once via another.
> 
> Check and you'll see - ppos is initialized with &iocb->ki_pos and never
> reassigned.  What happens is an equivalent of
> 
> int x = 0;
> int *p = &x;
> 
> x += 10;
> *p += 10;
> 
> which obviously ends with x increased by 20, not by 10...
> 
> 
I see. My mistake of ignoring this.
Thanks very much for spotting it out.