Re: [PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Adrian Hunter]

On 05/04/17 14:39, Vlastimil Babka wrote:
> On 04/05/2017 01:36 PM, Richard Weinberger wrote:
>> Michal,
>>
>> Am 05.04.2017 um 13:31 schrieb Michal Hocko:
>>> On Wed 05-04-17 09:47:00, Vlastimil Babka wrote:
 Nandsim has own functions set_memalloc() and clear_memalloc() for robust
 setting and clearing of PF_MEMALLOC. Replace them by the new generic 
 helpers.
 No functional change.
>>>
>>> This one smells like an abuser. Why the hell should read/write path
>>> touch memory reserves at all!
>>
>> Could be. Let's ask Adrian, AFAIK he wrote that code.
>> Adrian, can you please clarify why nandsim needs to play with PF_MEMALLOC?
> 
> I was thinking about it and concluded that since the simulator can be
> used as a block device where reclaimed pages go to, writing the data out
> is a memalloc operation. Then reading can be called as part of r-m-w
> cycle, so reading as well.

IIRC it was to avoid getting stuck with nandsim waiting on memory reclaim
and memory reclaim waiting on nandsim.

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Michal Hocko]

On Thu 06-04-17 09:33:44, Adrian Hunter wrote:
> On 05/04/17 14:39, Vlastimil Babka wrote:
> > On 04/05/2017 01:36 PM, Richard Weinberger wrote:
> >> Michal,
> >>
> >> Am 05.04.2017 um 13:31 schrieb Michal Hocko:
> >>> On Wed 05-04-17 09:47:00, Vlastimil Babka wrote:
>  Nandsim has own functions set_memalloc() and clear_memalloc() for robust
>  setting and clearing of PF_MEMALLOC. Replace them by the new generic 
>  helpers.
>  No functional change.
> >>>
> >>> This one smells like an abuser. Why the hell should read/write path
> >>> touch memory reserves at all!
> >>
> >> Could be. Let's ask Adrian, AFAIK he wrote that code.
> >> Adrian, can you please clarify why nandsim needs to play with PF_MEMALLOC?
> > 
> > I was thinking about it and concluded that since the simulator can be
> > used as a block device where reclaimed pages go to, writing the data out
> > is a memalloc operation. Then reading can be called as part of r-m-w
> > cycle, so reading as well.
> 
> IIRC it was to avoid getting stuck with nandsim waiting on memory reclaim
> and memory reclaim waiting on nandsim.

I've got lost in the indirection. Could you describe how would reclaim
get stuck waiting on these paths please?

-- 
Michal Hocko
SUSE Labs

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Michal Hocko]

On Wed 05-04-17 09:47:00, Vlastimil Babka wrote:
> Nandsim has own functions set_memalloc() and clear_memalloc() for robust
> setting and clearing of PF_MEMALLOC. Replace them by the new generic helpers.
> No functional change.

This one smells like an abuser. Why the hell should read/write path
touch memory reserves at all!

> 
> Signed-off-by: Vlastimil Babka 
> Cc: Boris Brezillon 
> Cc: Richard Weinberger 
> ---
>  drivers/mtd/nand/nandsim.c | 29 +
>  1 file changed, 9 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
> index cef818f535ed..03a0d057bf2f 100644
> --- a/drivers/mtd/nand/nandsim.c
> +++ b/drivers/mtd/nand/nandsim.c
> @@ -40,6 +40,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -1368,31 +1369,18 @@ static int get_pages(struct nandsim *ns, struct file 
> *file, size_t count, loff_t
>   return 0;
>  }
>  
> -static int set_memalloc(void)
> -{
> - if (current->flags & PF_MEMALLOC)
> - return 0;
> - current->flags |= PF_MEMALLOC;
> - return 1;
> -}
> -
> -static void clear_memalloc(int memalloc)
> -{
> - if (memalloc)
> - current->flags &= ~PF_MEMALLOC;
> -}
> -
>  static ssize_t read_file(struct nandsim *ns, struct file *file, void *buf, 
> size_t count, loff_t pos)
>  {
>   ssize_t tx;
> - int err, memalloc;
> + int err;
> + unsigned int noreclaim_flag;
>  
>   err = get_pages(ns, file, count, pos);
>   if (err)
>   return err;
> - memalloc = set_memalloc();
> + noreclaim_flag = memalloc_noreclaim_save();
>   tx = kernel_read(file, pos, buf, count);
> - clear_memalloc(memalloc);
> + memalloc_noreclaim_restore(noreclaim_flag);
>   put_pages(ns);
>   return tx;
>  }
> @@ -1400,14 +1388,15 @@ static ssize_t read_file(struct nandsim *ns, struct 
> file *file, void *buf, size_
>  static ssize_t write_file(struct nandsim *ns, struct file *file, void *buf, 
> size_t count, loff_t pos)
>  {
>   ssize_t tx;
> - int err, memalloc;
> + int err;
> + unsigned int noreclaim_flag;
>  
>   err = get_pages(ns, file, count, pos);
>   if (err)
>   return err;
> - memalloc = set_memalloc();
> + noreclaim_flag = memalloc_noreclaim_save();
>   tx = kernel_write(file, buf, count, pos);
> - clear_memalloc(memalloc);
> + memalloc_noreclaim_restore(noreclaim_flag);
>   put_pages(ns);
>   return tx;
>  }
> -- 
> 2.12.2

-- 
Michal Hocko
SUSE Labs

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

[PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Vlastimil Babka]

Nandsim has own functions set_memalloc() and clear_memalloc() for robust
setting and clearing of PF_MEMALLOC. Replace them by the new generic helpers.
No functional change.

Signed-off-by: Vlastimil Babka 
Cc: Boris Brezillon 
Cc: Richard Weinberger 
---
 drivers/mtd/nand/nandsim.c | 29 +
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/mtd/nand/nandsim.c b/drivers/mtd/nand/nandsim.c
index cef818f535ed..03a0d057bf2f 100644
--- a/drivers/mtd/nand/nandsim.c
+++ b/drivers/mtd/nand/nandsim.c
@@ -40,6 +40,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -1368,31 +1369,18 @@ static int get_pages(struct nandsim *ns, struct file 
*file, size_t count, loff_t
return 0;
 }
 
-static int set_memalloc(void)
-{
-   if (current->flags & PF_MEMALLOC)
-   return 0;
-   current->flags |= PF_MEMALLOC;
-   return 1;
-}
-
-static void clear_memalloc(int memalloc)
-{
-   if (memalloc)
-   current->flags &= ~PF_MEMALLOC;
-}
-
 static ssize_t read_file(struct nandsim *ns, struct file *file, void *buf, 
size_t count, loff_t pos)
 {
ssize_t tx;
-   int err, memalloc;
+   int err;
+   unsigned int noreclaim_flag;
 
err = get_pages(ns, file, count, pos);
if (err)
return err;
-   memalloc = set_memalloc();
+   noreclaim_flag = memalloc_noreclaim_save();
tx = kernel_read(file, pos, buf, count);
-   clear_memalloc(memalloc);
+   memalloc_noreclaim_restore(noreclaim_flag);
put_pages(ns);
return tx;
 }
@@ -1400,14 +1388,15 @@ static ssize_t read_file(struct nandsim *ns, struct 
file *file, void *buf, size_
 static ssize_t write_file(struct nandsim *ns, struct file *file, void *buf, 
size_t count, loff_t pos)
 {
ssize_t tx;
-   int err, memalloc;
+   int err;
+   unsigned int noreclaim_flag;
 
err = get_pages(ns, file, count, pos);
if (err)
return err;
-   memalloc = set_memalloc();
+   noreclaim_flag = memalloc_noreclaim_save();
tx = kernel_write(file, buf, count, pos);
-   clear_memalloc(memalloc);
+   memalloc_noreclaim_restore(noreclaim_flag);
put_pages(ns);
return tx;
 }
-- 
2.12.2

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Richard Weinberger]

Michal,

Am 05.04.2017 um 13:31 schrieb Michal Hocko:
> On Wed 05-04-17 09:47:00, Vlastimil Babka wrote:
>> Nandsim has own functions set_memalloc() and clear_memalloc() for robust
>> setting and clearing of PF_MEMALLOC. Replace them by the new generic helpers.
>> No functional change.
> 
> This one smells like an abuser. Why the hell should read/write path
> touch memory reserves at all!

Could be. Let's ask Adrian, AFAIK he wrote that code.
Adrian, can you please clarify why nandsim needs to play with PF_MEMALLOC?

Thanks,
//richard

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Michal Hocko]

On Wed 05-04-17 13:39:16, Vlastimil Babka wrote:
> On 04/05/2017 01:36 PM, Richard Weinberger wrote:
> > Michal,
> > 
> > Am 05.04.2017 um 13:31 schrieb Michal Hocko:
> >> On Wed 05-04-17 09:47:00, Vlastimil Babka wrote:
> >>> Nandsim has own functions set_memalloc() and clear_memalloc() for robust
> >>> setting and clearing of PF_MEMALLOC. Replace them by the new generic 
> >>> helpers.
> >>> No functional change.
> >>
> >> This one smells like an abuser. Why the hell should read/write path
> >> touch memory reserves at all!
> > 
> > Could be. Let's ask Adrian, AFAIK he wrote that code.
> > Adrian, can you please clarify why nandsim needs to play with PF_MEMALLOC?
> 
> I was thinking about it and concluded that since the simulator can be
> used as a block device where reclaimed pages go to, writing the data out
> is a memalloc operation. Then reading can be called as part of r-m-w
> cycle, so reading as well. But it would be great if somebody more
> knowledgeable confirmed this.

then this deserves a big fat comment explaining all the details,
including how the complete depletion of reserves is prevented.
-- 
Michal Hocko
SUSE Labs

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH 4/4] mtd: nand: nandsim: convert to memalloc_noreclaim_*()

[Vlastimil Babka]

On 04/05/2017 01:36 PM, Richard Weinberger wrote:
> Michal,
> 
> Am 05.04.2017 um 13:31 schrieb Michal Hocko:
>> On Wed 05-04-17 09:47:00, Vlastimil Babka wrote:
>>> Nandsim has own functions set_memalloc() and clear_memalloc() for robust
>>> setting and clearing of PF_MEMALLOC. Replace them by the new generic 
>>> helpers.
>>> No functional change.
>>
>> This one smells like an abuser. Why the hell should read/write path
>> touch memory reserves at all!
> 
> Could be. Let's ask Adrian, AFAIK he wrote that code.
> Adrian, can you please clarify why nandsim needs to play with PF_MEMALLOC?

I was thinking about it and concluded that since the simulator can be
used as a block device where reclaimed pages go to, writing the data out
is a memalloc operation. Then reading can be called as part of r-m-w
cycle, so reading as well. But it would be great if somebody more
knowledgeable confirmed this.

> Thanks,
> //richard
> 

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To post to this group, send email to open-iscsi@googlegroups.com.
Visit this group at https://groups.google.com/group/open-iscsi.
For more options, visit https://groups.google.com/d/optout.