Message ID | alpine.DEB.2.00.0912041519570.32367@hungry |
---|---|
State | Rejected |
Delegated to: | Andy Whitcroft |
Headers | show |
Manoj Iyer wrote: > ---- > resending patch, my previous mail bounced coz of membership requirement > ---- > > sync_page_range() was recently removed from 2.6, this causes the > iscsi-target build to fail in file-io.c. possibly sync_page_range() can be > replaced with generic_write_sync() ? > yeah they basically do the same thing though it looks like generic_write_sync can be heavier than what is needed in some cases. See http://lkml.org/lkml/2009/8/19/330 I haven't followed through to check whether this is the case with iscsi-target yet.
John Johansen wrote: > Manoj Iyer wrote: >> ---- >> resending patch, my previous mail bounced coz of membership requirement >> ---- >> >> sync_page_range() was recently removed from 2.6, this causes the >> iscsi-target build to fail in file-io.c. possibly sync_page_range() can be >> replaced with generic_write_sync() ? >> > yeah they basically do the same thing though it looks like generic_write_sync > can be heavier than what is needed in some cases. See > http://lkml.org/lkml/2009/8/19/330 > > I haven't followed through to check whether this is the case with iscsi-target > yet. > Seems to me that we should use filemap_write_and_wait_range() in order to preserve the intended semantics of the original call to sync_page_range(). Since this is a network protocol, we don't want file pages getting cached, or is my interpretation too naive? rtg
Tim Gardner wrote: > John Johansen wrote: >> Manoj Iyer wrote: >>> ---- >>> resending patch, my previous mail bounced coz of membership requirement >>> ---- >>> >>> sync_page_range() was recently removed from 2.6, this causes the >>> iscsi-target build to fail in file-io.c. possibly sync_page_range() can be >>> replaced with generic_write_sync() ? >>> >> yeah they basically do the same thing though it looks like generic_write_sync >> can be heavier than what is needed in some cases. See >> http://lkml.org/lkml/2009/8/19/330 >> >> I haven't followed through to check whether this is the case with iscsi-target >> yet. >> > > Seems to me that we should use filemap_write_and_wait_range() in order > to preserve the intended semantics of the original call to > sync_page_range(). Since this is a network protocol, we don't want file > pages getting cached, or is my interpretation too naive? > > rtg Andy - please pull from git://kernel.ubuntu.com/rtg/ubuntu-lucid iscsitarget. Use filemap_write_and_wait_range() in place of sync_page_range(). This appears to better preserve the original semantics. rtg
I did consider both functions, and I based my patch on SHAID aa3caafe53cab7ef60605e481cd5d7943e1c3022 pohmelfs: Use new syncing helper: Use new generic_write_sync() helper instead of sync_page_range() patch. I also saw a patch SHAID af0f4414f343429971d33b0dd8dccc85c1f3dcd2 xfs: Convert sync_page_range() to simple filemap_write_and_wait_range() From definition of generic_write_sync - perform syncing after a write if file / inode is sync seemed more appropriate to me at that time. Cheers --- manjo On Mon, 7 Dec 2009, Tim Gardner wrote: > Tim Gardner wrote: >> John Johansen wrote: >>> Manoj Iyer wrote: >>>> ---- >>>> resending patch, my previous mail bounced coz of membership requirement >>>> ---- >>>> >>>> sync_page_range() was recently removed from 2.6, this causes the >>>> iscsi-target build to fail in file-io.c. possibly sync_page_range() can be >>>> replaced with generic_write_sync() ? >>>> >>> yeah they basically do the same thing though it looks like generic_write_sync >>> can be heavier than what is needed in some cases. See >>> http://lkml.org/lkml/2009/8/19/330 >>> >>> I haven't followed through to check whether this is the case with iscsi-target >>> yet. >>> >> >> Seems to me that we should use filemap_write_and_wait_range() in order >> to preserve the intended semantics of the original call to >> sync_page_range(). Since this is a network protocol, we don't want file >> pages getting cached, or is my interpretation too naive? >> >> rtg > > Andy - please pull from git://kernel.ubuntu.com/rtg/ubuntu-lucid > iscsitarget. > > Use filemap_write_and_wait_range() in place of sync_page_range(). This > appears to better preserve the original semantics. > > rtg > -- > Tim Gardner tim.gardner@canonical.com >
Am Freitag, den 04.12.2009, 15:21 -0600 schrieb Manoj Iyer: > ---- > resending patch, my previous mail bounced coz of membership requirement > ---- > > sync_page_range() was recently removed from 2.6, this causes the > iscsi-target build to fail in file-io.c. possibly sync_page_range() can be > replaced with generic_write_sync() ? Thanks for the patch, but ultimately we replaced sync_page_range() with filemap_write_and_wait_range(). Thanks, Arne > Index: kernel/file-io.c > =================================================================== > --- kernel/file-io.c (revision 276) > +++ kernel/file-io.c (working copy) > @@ -75,8 +75,6 @@ > static int fileio_sync(struct iet_volume *lu, struct tio *tio) > { > struct fileio_data *p = lu->private; > - struct inode *inode = p->filp->f_dentry->d_inode; > - struct address_space *mapping = inode->i_mapping; > loff_t ppos, count; > int res; > > @@ -88,7 +86,7 @@ > count = lu->blk_cnt << lu->blk_shift; > } > > - res = sync_page_range(inode, mapping, ppos, count); > + res = generic_write_sync(p->filp, ppos, count); > if (res) { > eprintk("I/O error: syncing pages failed: %d\n", res); > return -EIO; > cheers > --- manjo > > > ------------------------------------------------------------------------------ > Join us December 9, 2009 for the Red Hat Virtual Experience, > a free event focused on virtualization and cloud computing. > Attend in-depth sessions from your desk. Your couch. Anywhere. > http://p.sf.net/sfu/redhat-sfdev2dev > _______________________________________________ > Iscsitarget-devel mailing list > Iscsitarget-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/iscsitarget-devel
Arne Redlich wrote: > Am Freitag, den 04.12.2009, 15:21 -0600 schrieb Manoj Iyer: >> ---- >> resending patch, my previous mail bounced coz of membership requirement >> ---- >> >> sync_page_range() was recently removed from 2.6, this causes the >> iscsi-target build to fail in file-io.c. possibly sync_page_range() can be >> replaced with generic_write_sync() ? > > Thanks for the patch, but ultimately we replaced sync_page_range() with > filemap_write_and_wait_range(). > > Thanks, > Arne filemap_write_and_wait_range() is the same conclusion that I came to when updating this driver for 2.6.32. Have you attempted to get this driver included in the mainline kernel? rtg
Index: kernel/file-io.c =================================================================== --- kernel/file-io.c (revision 276) +++ kernel/file-io.c (working copy) @@ -75,8 +75,6 @@ static int fileio_sync(struct iet_volume *lu, struct tio *tio) { struct fileio_data *p = lu->private; - struct inode *inode = p->filp->f_dentry->d_inode; - struct address_space *mapping = inode->i_mapping; loff_t ppos, count; int res; @@ -88,7 +86,7 @@ count = lu->blk_cnt << lu->blk_shift; } - res = sync_page_range(inode, mapping, ppos, count); + res = generic_write_sync(p->filp, ppos, count); if (res) { eprintk("I/O error: syncing pages failed: %d\n", res); return -EIO;