From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Morton Subject: Re: fs requirements for loop device? Date: Mon, 13 May 2002 16:47:39 -0700 Sender: linux-fsdevel-owner@vger.kernel.org Message-ID: <3CE0509B.478F6D18@zip.com.au> References: <3CE0337D.AFF02E2@zip.com.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: Christoph Hellwig , Steve Lord , Anton Altaparmakov , linux-fsdevel@vger.kernel.org Return-path: To: Urban Widmark List-Id: linux-fsdevel.vger.kernel.org Urban Widmark wrote: > > On Mon, 13 May 2002, Andrew Morton wrote: > > > Can anyone point at a reason why loop shouldn't use the > > ->read() and ->write() methods of the backing file's > > file_operations? > > The patch below is my naive implementation of that for 2.4.18. > > ... loop.c | 113 +++++++++++------------------------------------------------------ 1 files changed, 20 insertions, 93 deletions that's a good sign ;) > ... > + fs = get_fs(); > + set_fs(get_ds()); > + ret = file->f_op->write(file, data, len, &pos); > + set_fs(fs); f_op->write() returns the number of bytes written, or a negative errno. So you'll need if (ret > 0) ret = 0; in there. Also it may be nicer to copy `pos' into a local when you call ->read and ->write, just to avoid altering your callers' locals, and potentially surprising them (it's OK at present, so minor point). > ... > + /* This looks strange, if someone free's lo won't they also fput > + lo_backing_file? */ > spin_lock_irq(&lo->lo_lock); > file = lo->lo_backing_file; > spin_unlock_irq(&lo->lo_lock); They do. loop_clr_fd() does fput(). If this code is trying to actually do something legitimate then clearly it is doing it wrongly. Just delete the locking I think. This is a big improvement to loop. Let's try to get this into 2.4.20. -