From mboxrd@z Thu Jan 1 00:00:00 1970 From: Changli Gao Subject: Re: [PATCH v2 4/4] splice: fix updating sd->pos wrongly Date: Fri, 28 May 2010 19:36:36 +0800 Message-ID: References: <1274885083-15735-1-git-send-email-xiaosuo@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: axboe@kernel.dk, viro@zeniv.linux.org.uk, mszeredi@suse.cz, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Miklos Szeredi Return-path: In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-Id: linux-fsdevel.vger.kernel.org On Fri, May 28, 2010 at 7:11 PM, Miklos Szeredi wro= te: > On Fri, 28 May 2010, Changli Gao wrote: >> =C2=A0Before checking the sd->pos, sd->pos >> already is updated with the value returned by splice_read(), so if i= n >> file is seekabble, sd->pos is non-zero when I checking it. > > I see the logic now. =C2=A0But it's really confusing, at least some > comments would be needed. > > But I think it would be better not to try reusing sd->pos for reading= , > but supplying an "loff_t *read_ppos" to splice_direct_to_actor and > leaving sd->pos alone (for the write side to use and update as > necessary). > if (ret < read_len) { I think is much like a hack too. :) - sd->pos =3D prev_pos + ret; + if (sd->pos) + sd->pos =3D prev_pos + ret; goto out_release; } Anyway, it can fix the issue I mentioned above. Any future cleanup should not be mixed in, IMO. --=20 Regards=EF=BC=8C Changli Gao(xiaosuo@gmail.com)