From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jamie Lokier Subject: Re: [PATCH v2 4/4] splice: fix updating sd->pos wrongly Date: Sat, 29 May 2010 15:09:01 +0100 Message-ID: <20100529140901.GD3106@shareable.org> References: <1274885083-15735-1-git-send-email-xiaosuo@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Cc: Miklos Szeredi , axboe@kernel.dk, viro@zeniv.linux.org.uk, mszeredi@suse.cz, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org To: Changli Gao Return-path: Received: from mail2.shareable.org ([80.68.89.115]:39127 "EHLO mail2.shareable.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752106Ab0E2OJI (ORCPT ); Sat, 29 May 2010 10:09:08 -0400 Content-Disposition: inline In-Reply-To: Sender: linux-fsdevel-owner@vger.kernel.org List-ID: Changli Gao wrote: > On Fri, May 28, 2010 at 6:13 PM, Miklos Szeredi w= rote: > > On Fri, 28 May 2010, Changli Gao wrote: > >> On Fri, May 28, 2010 at 5:42 PM, Miklos Szeredi wrote: > >> > On Wed, 26 May 2010, Changli Gao wrote: > >> >> fix updating sd->pos wrongly. > >> >> > >> >> In error path, we don't need to updating sd->pos, if the file i= sn't seekable. > >> > > >> > This patch is nonsense. =A0Why should we handle sd->pos !=3D 0 c= ase > >> > differently? > >> > > >> > >> If the in file isn't seekable, its splice_read won't update *ppos,= so > >> in the error path, we'd better not change it too. Otherwise, some > >> assumption will go wrong. > > > > That may be true, but the patch is still nonsense. > > > > Look, your patch is updating/not updating sd->pos based on whether = it > > is zero or not. =A0It will prevent updating the position for socket= s, > > but it will also prevent updating the position for regular files if > > the position is zero, which is really not what we want. >=20 > I think you misread my patch. Before checking the sd->pos, sd->pos > already is updated with the value returned by splice_read(), so if in > file is seekabble, sd->pos is non-zero when I checking it. Not true if the "file" is /dev/mem or /dev/kmem. The starting offset can be negative, so can end at zero. It's an obscure case and I don't know if you can sendfile from /dev/{,k}mem anyway :-) but illustrates why sd->pos !=3D 0 is dubious. -- Jamie -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel= " in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html