replace sync_page_range() with generic_write_sync() in file-io.c

----
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() ?

cheers
--- manjo

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