From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from psmtp.com (na3sys010amx153.postini.com [74.125.245.153]) by kanga.kvack.org (Postfix) with SMTP id BD3BC6B00ED for ; Tue, 8 May 2012 02:28:27 -0400 (EDT) Received: by vbbey12 with SMTP id ey12so5886776vbb.14 for ; Mon, 07 May 2012 23:28:26 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20120506233117.GU5091@dastard> References: <1336054995-22988-1-git-send-email-svenkatr@ti.com> <1336054995-22988-2-git-send-email-svenkatr@ti.com> <20120506233117.GU5091@dastard> From: mani Date: Tue, 8 May 2012 11:58:06 +0530 Message-ID: Subject: Re: [PATCH v2 01/16] FS: Added demand paging markers to filesystem Content-Type: multipart/alternative; boundary=20cf3079b7fc8a3fec04bf807ef7 Sender: owner-linux-mm@kvack.org List-ID: To: Dave Chinner Cc: Venkatraman S , linux-mmc@vger.kernel.org, cjb@laptop.org, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-omap@vger.kernel.org, linux-kernel@vger.kernel.org, arnd.bergmann@linaro.org, alex.lemberg@sandisk.com, ilan.smith@sandisk.com, lporzio@micron.com, rmk+kernel@arm.linux.org.uk --20cf3079b7fc8a3fec04bf807ef7 Content-Type: text/plain; charset=ISO-8859-1 How about adding the AS_DMPG flag in the file -> address_space when getting a filemap_fault() so that we can treat the page fault pages as the high priority pages over normal read requests. How about changing below lines for the support of the pages those are requested for the page fault ? --- a/fs/mpage.c 2012-05-04 12:59:12.000000000 +0530 +++ b/fs/mpage.c 2012-05-07 13:13:49.000000000 +0530 @@ -408,6 +408,8 @@ mpage_readpages(struct address_space *ma &last_block_in_bio, &map_bh, &first_logical_block, get_block); + if(test_bit(AS_DMPG, &mapping->flags) && bio) + bio->bi_rw |= REQ_RW_DMPG } page_cache_release(page); } --- a/include/linux/pagemap.h 2012-05-04 12:57:35.000000000 +0530 +++ b/include/linux/pagemap.h 2012-05-07 13:15:24.000000000 +0530 @@ -27,6 +27,7 @@ enum mapping_flags { #if defined (CONFIG_BD_CACHE_ENABLED) AS_DIRECT = __GFP_BITS_SHIFT + 4, /* DIRECT_IO specified on file op */ #endif + AS_DMPG = __GFP_BITS_SHIFT + 5, /* DEMAND PAGE specified on file op */ }; static inline void mapping_set_error(struct address_space *mapping, int error) --- a/mm/filemap.c 2012-05-04 12:58:49.000000000 +0530 +++ b/mm/filemap.c 2012-05-07 13:15:03.000000000 +0530 @@ -1646,6 +1646,7 @@ int filemap_fault(struct vm_area_struct if (offset >= size) return VM_FAULT_SIGBUS; + set_bit(AS_DMPG, &file->f_mapping->flags); /* * Do we have something in the page cache already? */ Will these changes have any adverse effect ? Thanks & Regards Manish On Mon, May 7, 2012 at 5:01 AM, Dave Chinner wrote: > On Thu, May 03, 2012 at 07:53:00PM +0530, Venkatraman S wrote: > > From: Ilan Smith > > > > Add attribute to identify demand paging requests. > > Mark readpages with demand paging attribute. > > > > Signed-off-by: Ilan Smith > > Signed-off-by: Alex Lemberg > > Signed-off-by: Venkatraman S > > --- > > fs/mpage.c | 2 ++ > > include/linux/bio.h | 7 +++++++ > > include/linux/blk_types.h | 2 ++ > > 3 files changed, 11 insertions(+) > > > > diff --git a/fs/mpage.c b/fs/mpage.c > > index 0face1c..8b144f5 100644 > > --- a/fs/mpage.c > > +++ b/fs/mpage.c > > @@ -386,6 +386,8 @@ mpage_readpages(struct address_space *mapping, > struct list_head *pages, > > &last_block_in_bio, &map_bh, > > &first_logical_block, > > get_block); > > + if (bio) > > + bio->bi_rw |= REQ_RW_DMPG; > > Have you thought about the potential for DOSing a machine > with this? That is, user data reads can now preempt writes of any > kind, effectively stalling writeback and memory reclaim which will > lead to OOM situations. Or, alternatively, journal flushing will get > stalled and no new modifications can take place until the read > stream stops. > > This really seems like functionality that belongs in an IO > scheduler so that write starvation can be avoided, not in high-level > data read paths where we have no clue about anything else going on > in the IO subsystem.... > > Cheers, > > Dave. > -- > Dave Chinner > david@fromorbit.com > -- > To unsubscribe from this list: send the line "unsubscribe linux-mmc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > --20cf3079b7fc8a3fec04bf807ef7 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable How about adding the AS_DMPG flag in the file -> address_space when gett= ing a filemap_fault()
so that we can treat the page fault pages as the = high priority pages over normal read requests.
How about changing below = lines for the support of the pages those are requested for the page fault ?=


--- a/fs/mpage.c 2012-05-04 12:59:12.000000000 +0530
+++ b/fs/mp= age.c 2012-05-07 13:13:49.000000000 +0530
@@ -408,6 +408,8 @@ mpage_read= pages(struct address_space *ma
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0 &last_block_in_bio, &map_bh,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 &first_logica= l_block,
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 get_b= lock);
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 if(test_bit(AS_DMPG, &mapping= ->flags) && bio)
+=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0 bio->bi_rw |=3D REQ_RW_DMPG
=A0=A0=A0=A0=A0=A0=A0 }
=A0=A0=A0=A0=A0=A0=A0 page_cache_release(page);
=A0=A0=A0 }
--- a/inc= lude/linux/pagemap.h=A0=A0=A0 2012-05-04 12:57:35.000000000 +0530
+++ b/= include/linux/pagemap.h=A0=A0=A0 2012-05-07 13:15:24.000000000 +0530
@@ = -27,6 +27,7 @@ enum mapping_flags {
=A0#if defined (CONFIG_BD_CACHE_ENABLED)
=A0=A0=A0 AS_DIRECT=A0 =3D=A0= =A0 __GFP_BITS_SHIFT + 4,=A0 /* DIRECT_IO specified on file op */
=A0#en= dif
+=A0=A0 AS_DMPG=A0 =3D=A0=A0 __GFP_BITS_SHIFT + 5,=A0 /* DEMAND PAGE= specified on file op */
=A0};
=A0
=A0static inline void mapping_set_error(struct address_space *mappin= g, int error)

--- a/mm/filemap.c=A0=A0 2012-05-04 12:58:49.000000000= +0530
+++ b/mm/filemap.c=A0=A0 2012-05-07 13:15:03.000000000 +0530
@= @ -1646,6 +1646,7 @@ int filemap_fault(struct vm_area_struct
=A0=A0=A0 if (offset >=3D size)
=A0=A0=A0=A0=A0=A0=A0 return VM_FAULT= _SIGBUS;
=A0
+=A0=A0 set_bit(AS_DMPG, &file->f_mapping->fla= gs);
=A0=A0=A0 /*
=A0=A0=A0=A0 * Do we have something in the page cac= he already?
=A0=A0=A0=A0 */
=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0= =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0
Will these changes have any adverse effect ?

Thanks & RegardsManish

On Mon, May 7, 2012 at 5:01 AM, = Dave Chinner <david@fromorbit.com> wrote:
On Thu, May 03, 2012 at 07= :53:00PM +0530, Venkatraman S wrote:
> From: Ilan Smith <ilan.sm= ith@sandisk.com>
>
> Add attribute to identify demand paging requests.
> Mark readpages with demand paging attribute.
>
> Signed-off-by: Ilan Smith <ilan.smith@sandisk.com>
> Signed-off-by: Alex Lemberg <alex.lemberg@sandisk.com>
> Signed-off-by: Venkatraman S <sv= enkatr@ti.com>
> ---
> =A0fs/mpage.c =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0| =A0 =A02 ++
> =A0include/linux/bio.h =A0 =A0 =A0 | =A0 =A07 +++++++
> =A0include/linux/blk_types.h | =A0 =A02 ++
> =A03 files changed, 11 insertions(+)
>
> diff --git a/fs/mpage.c b/fs/mpage.c
> index 0face1c..8b144f5 100644
> --- a/fs/mpage.c
> +++ b/fs/mpage.c
> @@ -386,6 +386,8 @@ mpage_readpages(struct address_space *mapping, str= uct list_head *pages,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 &last_block_in_bio, &map_bh,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 &first_logical_block,
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 get_block);
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (bio)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bio->bi_r= w |=3D REQ_RW_DMPG;

Have you thought about the potential for DOSing a machine
with this? That is, user data reads can now preempt writes of any
kind, effectively stalling writeback and memory reclaim which will
lead to OOM situations. Or, alternatively, journal flushing will get
stalled and no new modifications can take place until the read
stream stops.

This really seems like functionality that belongs in an IO
scheduler so that write starvation can be avoided, not in high-level
data read paths where we have no clue about anything else going on
in the IO subsystem....

Cheers,

Dave.
--
Dave Chinner
david@fromorbit.com
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc&qu= ot; in
the body of a message to major= domo@vger.kernel.org
More majordomo info at =A0http://vger.kernel.org/majordomo-info.html

--20cf3079b7fc8a3fec04bf807ef7-- -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: email@kvack.org