linux-next: Signed-off-by missing for commits in the gfs2 tree

[Stephen Rothwell]

Hi all,

Commits

  cfdee655b178 ("GFS2: Make height info part of metapath")
  8bdecc2563a5 ("GFS2: flush the log and all pages for jdata as we do for 
WB_SYNC_ALL")
  1ecfcbf9a59e ("GFS2: Take inode off order_write list when setting jdata flag")

are missing a Signed-off-by from their committer.

-- 
Cheers,
Stephen Rothwell

linux-next: Signed-off-by missing for commits in the gfs2 tree

[Stephen Rothwell]

Hi all,

Commits

  cfdee655b178 ("GFS2: Make height info part of metapath")
  8bdecc2563a5 ("GFS2: flush the log and all pages for jdata as we do for 
WB_SYNC_ALL")
  1ecfcbf9a59e ("GFS2: Take inode off order_write list when setting jdata flag")

are missing a Signed-off-by from their committer.

-- 
Cheers,
Stephen Rothwell

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Huang, Ying]

Minchan Kim  writes:

> Hi Huang,
>
> On Tue, Oct 31, 2017 at 01:32:32PM +0800, Huang, Ying wrote:
>> Hi, Minchan,
>> 
>> Minchan Kim  writes:
>> 
>> > Hi Huang,
>> >
>> > On Fri, Oct 27, 2017 at 01:53:27PM +0800, Huang, Ying wrote:
>> >> From: Huang Ying 
>> >> 
>> >> When a page fault occurs for a swap entry, the physical swap readahead
>> >> (not the VMA base swap readahead) may readahead several swap entries
>> >> after the fault swap entry.  The readahead algorithm calculates some
>> >> of the swap entries to readahead via increasing the offset of the
>> >> fault swap entry without checking whether they are beyond the end of
>> >> the swap device and it relys on the __swp_swapcount() and
>> >> swapcache_prepare() to check it.  Although __swp_swapcount() checks
>> >> for the swap entry passed in, it will complain with the error message
>> >> as follow for the expected invalid swap entry.  This may make the end
>> >> users confused.
>> >> 
>> >>   swap_info_get: Bad swap offset entry 0200f8a7
>> >> 
>> >> To fix the false error message, the swap entry checking is added in
>> >> swap readahead to avoid to pass the out-bound swap entries and the
>> >> swap entry reserved for the swap header to __swp_swapcount() and
>> >> swapcache_prepare().
>> >> 
>> >> Cc: Tim Chen 
>> >> Cc: Minchan Kim 
>> >> Cc: Michal Hocko 
>> >> Cc:  # 4.11-4.13
>> >> Reported-by: Christian Kujau 
>> >> Fixes: e8c26ab60598 ("mm/swap: skip readahead for unreferenced swap 
>> >> slots")
>> >> Signed-off-by: "Huang, Ying" 
>> >> ---
>> >>  include/linux/swap.h |  1 +
>> >>  mm/swap_state.c  |  6 --
>> >>  mm/swapfile.c| 21 +
>> >>  3 files changed, 26 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> >> index 84255b3da7c1..43b4b821c805 100644
>> >> --- a/include/linux/swap.h
>> >> +++ b/include/linux/swap.h
>> >> @@ -476,6 +476,7 @@ extern int page_swapcount(struct page *);
>> >>  extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
>> >>  extern int __swp_swapcount(swp_entry_t entry);
>> >>  extern int swp_swapcount(swp_entry_t entry);
>> >> +extern bool swap_entry_check(swp_entry_t entry);
>> >>  extern struct swap_info_struct *page_swap_info(struct page *);
>> >>  extern struct swap_info_struct *swp_swap_info(swp_entry_t entry);
>> >>  extern bool reuse_swap_page(struct page *, int *);
>> >> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> >> index 6c017ced11e6..7dd70e77058d 100644
>> >> --- a/mm/swap_state.c
>> >> +++ b/mm/swap_state.c
>> >> @@ -569,11 +569,13 @@ struct page *swapin_readahead(swp_entry_t entry, 
>> >> gfp_t gfp_mask,
>> >>   /* Read a page_cluster sized and aligned cluster around offset. */
>> >>   start_offset = offset & ~mask;
>> >>   end_offset = offset | mask;
>> >> - if (!start_offset)  /* First page is swap header. */
>> >> - start_offset++;
>> >>  
>> >>   blk_start_plug();
>> >>   for (offset = start_offset; offset <= end_offset ; offset++) {
>> >> + swp_entry_t ent = swp_entry(swp_type(entry), offset);
>> >> +
>> >> + if (!swap_entry_check(ent))
>> >> + continue;
>> >>   /* Ok, do the async read-ahead now */
>> >>   page = __read_swap_cache_async(
>> >>   swp_entry(swp_type(entry), offset),
>> >> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> >> index 3074b02eaa09..b04cec29c234 100644
>> >> --- a/mm/swapfile.c
>> >> +++ b/mm/swapfile.c
>> >> @@ -1107,6 +1107,27 @@ static struct swap_info_struct 
>> >> *swap_info_get_cont(swp_entry_t entry,
>> >>   return p;
>> >>  }
>> >>  
>> >> +bool swap_entry_check(swp_entry_t entry)
>> >> +{
>> >> + struct swap_info_struct *p;
>> >> + unsigned long offset, type;
>> >> +
>> >> + type = swp_type(entry);
>> >> + if (type >= nr_swapfiles)
>> >> + goto bad_file;
>> >> + p = swap_info[type];
>> >> + offset = swp_offset(entry);
>> >> + if (unlikely(!offset || offset >= p->max))
>> >> + goto out;
>> >> +
>> >> + return true;
>> >> +
>> >> +bad_file:
>> >> + pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
>> >> +out:
>> >> + return false;
>> >> +}
>> >> +
>> >>  static unsigned char __swap_entry_free(struct swap_info_struct *p,
>> >>  swp_entry_t entry, unsigned char usage)
>> >>  {
>> >> -- 
>> >> 2.14.2
>> >
>> > Although it's better than old, we can make it simple, still.
>> >
>> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
>> > index 291c4b534658..f50d5a48f03a 100644
>> > --- a/include/linux/swapops.h
>> > +++ b/include/linux/swapops.h
>> > @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
>> >return (entry.val >> SWP_TYPE_SHIFT(entry));
>> >  }
>> >  
>> > +extern struct swap_info_struct 

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Huang, Ying]

Minchan Kim  writes:

> Hi Huang,
>
> On Tue, Oct 31, 2017 at 01:32:32PM +0800, Huang, Ying wrote:
>> Hi, Minchan,
>> 
>> Minchan Kim  writes:
>> 
>> > Hi Huang,
>> >
>> > On Fri, Oct 27, 2017 at 01:53:27PM +0800, Huang, Ying wrote:
>> >> From: Huang Ying 
>> >> 
>> >> When a page fault occurs for a swap entry, the physical swap readahead
>> >> (not the VMA base swap readahead) may readahead several swap entries
>> >> after the fault swap entry.  The readahead algorithm calculates some
>> >> of the swap entries to readahead via increasing the offset of the
>> >> fault swap entry without checking whether they are beyond the end of
>> >> the swap device and it relys on the __swp_swapcount() and
>> >> swapcache_prepare() to check it.  Although __swp_swapcount() checks
>> >> for the swap entry passed in, it will complain with the error message
>> >> as follow for the expected invalid swap entry.  This may make the end
>> >> users confused.
>> >> 
>> >>   swap_info_get: Bad swap offset entry 0200f8a7
>> >> 
>> >> To fix the false error message, the swap entry checking is added in
>> >> swap readahead to avoid to pass the out-bound swap entries and the
>> >> swap entry reserved for the swap header to __swp_swapcount() and
>> >> swapcache_prepare().
>> >> 
>> >> Cc: Tim Chen 
>> >> Cc: Minchan Kim 
>> >> Cc: Michal Hocko 
>> >> Cc:  # 4.11-4.13
>> >> Reported-by: Christian Kujau 
>> >> Fixes: e8c26ab60598 ("mm/swap: skip readahead for unreferenced swap 
>> >> slots")
>> >> Signed-off-by: "Huang, Ying" 
>> >> ---
>> >>  include/linux/swap.h |  1 +
>> >>  mm/swap_state.c  |  6 --
>> >>  mm/swapfile.c| 21 +
>> >>  3 files changed, 26 insertions(+), 2 deletions(-)
>> >> 
>> >> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> >> index 84255b3da7c1..43b4b821c805 100644
>> >> --- a/include/linux/swap.h
>> >> +++ b/include/linux/swap.h
>> >> @@ -476,6 +476,7 @@ extern int page_swapcount(struct page *);
>> >>  extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
>> >>  extern int __swp_swapcount(swp_entry_t entry);
>> >>  extern int swp_swapcount(swp_entry_t entry);
>> >> +extern bool swap_entry_check(swp_entry_t entry);
>> >>  extern struct swap_info_struct *page_swap_info(struct page *);
>> >>  extern struct swap_info_struct *swp_swap_info(swp_entry_t entry);
>> >>  extern bool reuse_swap_page(struct page *, int *);
>> >> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> >> index 6c017ced11e6..7dd70e77058d 100644
>> >> --- a/mm/swap_state.c
>> >> +++ b/mm/swap_state.c
>> >> @@ -569,11 +569,13 @@ struct page *swapin_readahead(swp_entry_t entry, 
>> >> gfp_t gfp_mask,
>> >>   /* Read a page_cluster sized and aligned cluster around offset. */
>> >>   start_offset = offset & ~mask;
>> >>   end_offset = offset | mask;
>> >> - if (!start_offset)  /* First page is swap header. */
>> >> - start_offset++;
>> >>  
>> >>   blk_start_plug();
>> >>   for (offset = start_offset; offset <= end_offset ; offset++) {
>> >> + swp_entry_t ent = swp_entry(swp_type(entry), offset);
>> >> +
>> >> + if (!swap_entry_check(ent))
>> >> + continue;
>> >>   /* Ok, do the async read-ahead now */
>> >>   page = __read_swap_cache_async(
>> >>   swp_entry(swp_type(entry), offset),
>> >> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> >> index 3074b02eaa09..b04cec29c234 100644
>> >> --- a/mm/swapfile.c
>> >> +++ b/mm/swapfile.c
>> >> @@ -1107,6 +1107,27 @@ static struct swap_info_struct 
>> >> *swap_info_get_cont(swp_entry_t entry,
>> >>   return p;
>> >>  }
>> >>  
>> >> +bool swap_entry_check(swp_entry_t entry)
>> >> +{
>> >> + struct swap_info_struct *p;
>> >> + unsigned long offset, type;
>> >> +
>> >> + type = swp_type(entry);
>> >> + if (type >= nr_swapfiles)
>> >> + goto bad_file;
>> >> + p = swap_info[type];
>> >> + offset = swp_offset(entry);
>> >> + if (unlikely(!offset || offset >= p->max))
>> >> + goto out;
>> >> +
>> >> + return true;
>> >> +
>> >> +bad_file:
>> >> + pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
>> >> +out:
>> >> + return false;
>> >> +}
>> >> +
>> >>  static unsigned char __swap_entry_free(struct swap_info_struct *p,
>> >>  swp_entry_t entry, unsigned char usage)
>> >>  {
>> >> -- 
>> >> 2.14.2
>> >
>> > Although it's better than old, we can make it simple, still.
>> >
>> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
>> > index 291c4b534658..f50d5a48f03a 100644
>> > --- a/include/linux/swapops.h
>> > +++ b/include/linux/swapops.h
>> > @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
>> >return (entry.val >> SWP_TYPE_SHIFT(entry));
>> >  }
>> >  
>> > +extern struct swap_info_struct *swap_info[];
>> > +
>> > +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
>> > +{
>> > +  return swap_info[swp_type(entry)];
>> > +}
>> > +
>> 
>> Why not just use swp_swap_info()?
>
> 

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Minchan Kim]

Hi Huang,

On Tue, Oct 31, 2017 at 01:32:32PM +0800, Huang, Ying wrote:
> Hi, Minchan,
> 
> Minchan Kim  writes:
> 
> > Hi Huang,
> >
> > On Fri, Oct 27, 2017 at 01:53:27PM +0800, Huang, Ying wrote:
> >> From: Huang Ying 
> >> 
> >> When a page fault occurs for a swap entry, the physical swap readahead
> >> (not the VMA base swap readahead) may readahead several swap entries
> >> after the fault swap entry.  The readahead algorithm calculates some
> >> of the swap entries to readahead via increasing the offset of the
> >> fault swap entry without checking whether they are beyond the end of
> >> the swap device and it relys on the __swp_swapcount() and
> >> swapcache_prepare() to check it.  Although __swp_swapcount() checks
> >> for the swap entry passed in, it will complain with the error message
> >> as follow for the expected invalid swap entry.  This may make the end
> >> users confused.
> >> 
> >>   swap_info_get: Bad swap offset entry 0200f8a7
> >> 
> >> To fix the false error message, the swap entry checking is added in
> >> swap readahead to avoid to pass the out-bound swap entries and the
> >> swap entry reserved for the swap header to __swp_swapcount() and
> >> swapcache_prepare().
> >> 
> >> Cc: Tim Chen 
> >> Cc: Minchan Kim 
> >> Cc: Michal Hocko 
> >> Cc:  # 4.11-4.13
> >> Reported-by: Christian Kujau 
> >> Fixes: e8c26ab60598 ("mm/swap: skip readahead for unreferenced swap slots")
> >> Signed-off-by: "Huang, Ying" 
> >> ---
> >>  include/linux/swap.h |  1 +
> >>  mm/swap_state.c  |  6 --
> >>  mm/swapfile.c| 21 +
> >>  3 files changed, 26 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> index 84255b3da7c1..43b4b821c805 100644
> >> --- a/include/linux/swap.h
> >> +++ b/include/linux/swap.h
> >> @@ -476,6 +476,7 @@ extern int page_swapcount(struct page *);
> >>  extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
> >>  extern int __swp_swapcount(swp_entry_t entry);
> >>  extern int swp_swapcount(swp_entry_t entry);
> >> +extern bool swap_entry_check(swp_entry_t entry);
> >>  extern struct swap_info_struct *page_swap_info(struct page *);
> >>  extern struct swap_info_struct *swp_swap_info(swp_entry_t entry);
> >>  extern bool reuse_swap_page(struct page *, int *);
> >> diff --git a/mm/swap_state.c b/mm/swap_state.c
> >> index 6c017ced11e6..7dd70e77058d 100644
> >> --- a/mm/swap_state.c
> >> +++ b/mm/swap_state.c
> >> @@ -569,11 +569,13 @@ struct page *swapin_readahead(swp_entry_t entry, 
> >> gfp_t gfp_mask,
> >>/* Read a page_cluster sized and aligned cluster around offset. */
> >>start_offset = offset & ~mask;
> >>end_offset = offset | mask;
> >> -  if (!start_offset)  /* First page is swap header. */
> >> -  start_offset++;
> >>  
> >>blk_start_plug();
> >>for (offset = start_offset; offset <= end_offset ; offset++) {
> >> +  swp_entry_t ent = swp_entry(swp_type(entry), offset);
> >> +
> >> +  if (!swap_entry_check(ent))
> >> +  continue;
> >>/* Ok, do the async read-ahead now */
> >>page = __read_swap_cache_async(
> >>swp_entry(swp_type(entry), offset),
> >> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >> index 3074b02eaa09..b04cec29c234 100644
> >> --- a/mm/swapfile.c
> >> +++ b/mm/swapfile.c
> >> @@ -1107,6 +1107,27 @@ static struct swap_info_struct 
> >> *swap_info_get_cont(swp_entry_t entry,
> >>return p;
> >>  }
> >>  
> >> +bool swap_entry_check(swp_entry_t entry)
> >> +{
> >> +  struct swap_info_struct *p;
> >> +  unsigned long offset, type;
> >> +
> >> +  type = swp_type(entry);
> >> +  if (type >= nr_swapfiles)
> >> +  goto bad_file;
> >> +  p = swap_info[type];
> >> +  offset = swp_offset(entry);
> >> +  if (unlikely(!offset || offset >= p->max))
> >> +  goto out;
> >> +
> >> +  return true;
> >> +
> >> +bad_file:
> >> +  pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
> >> +out:
> >> +  return false;
> >> +}
> >> +
> >>  static unsigned char __swap_entry_free(struct swap_info_struct *p,
> >>   swp_entry_t entry, unsigned char usage)
> >>  {
> >> -- 
> >> 2.14.2
> >
> > Although it's better than old, we can make it simple, still.
> >
> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > index 291c4b534658..f50d5a48f03a 100644
> > --- a/include/linux/swapops.h
> > +++ b/include/linux/swapops.h
> > @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
> > return (entry.val >> SWP_TYPE_SHIFT(entry));
> >  }
> >  
> > +extern struct swap_info_struct *swap_info[];
> > +
> > +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
> > +{
> > +   return swap_info[swp_type(entry)];
> > +}

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Minchan Kim]

Hi Huang,

On Tue, Oct 31, 2017 at 01:32:32PM +0800, Huang, Ying wrote:
> Hi, Minchan,
> 
> Minchan Kim  writes:
> 
> > Hi Huang,
> >
> > On Fri, Oct 27, 2017 at 01:53:27PM +0800, Huang, Ying wrote:
> >> From: Huang Ying 
> >> 
> >> When a page fault occurs for a swap entry, the physical swap readahead
> >> (not the VMA base swap readahead) may readahead several swap entries
> >> after the fault swap entry.  The readahead algorithm calculates some
> >> of the swap entries to readahead via increasing the offset of the
> >> fault swap entry without checking whether they are beyond the end of
> >> the swap device and it relys on the __swp_swapcount() and
> >> swapcache_prepare() to check it.  Although __swp_swapcount() checks
> >> for the swap entry passed in, it will complain with the error message
> >> as follow for the expected invalid swap entry.  This may make the end
> >> users confused.
> >> 
> >>   swap_info_get: Bad swap offset entry 0200f8a7
> >> 
> >> To fix the false error message, the swap entry checking is added in
> >> swap readahead to avoid to pass the out-bound swap entries and the
> >> swap entry reserved for the swap header to __swp_swapcount() and
> >> swapcache_prepare().
> >> 
> >> Cc: Tim Chen 
> >> Cc: Minchan Kim 
> >> Cc: Michal Hocko 
> >> Cc:  # 4.11-4.13
> >> Reported-by: Christian Kujau 
> >> Fixes: e8c26ab60598 ("mm/swap: skip readahead for unreferenced swap slots")
> >> Signed-off-by: "Huang, Ying" 
> >> ---
> >>  include/linux/swap.h |  1 +
> >>  mm/swap_state.c  |  6 --
> >>  mm/swapfile.c| 21 +
> >>  3 files changed, 26 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/include/linux/swap.h b/include/linux/swap.h
> >> index 84255b3da7c1..43b4b821c805 100644
> >> --- a/include/linux/swap.h
> >> +++ b/include/linux/swap.h
> >> @@ -476,6 +476,7 @@ extern int page_swapcount(struct page *);
> >>  extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
> >>  extern int __swp_swapcount(swp_entry_t entry);
> >>  extern int swp_swapcount(swp_entry_t entry);
> >> +extern bool swap_entry_check(swp_entry_t entry);
> >>  extern struct swap_info_struct *page_swap_info(struct page *);
> >>  extern struct swap_info_struct *swp_swap_info(swp_entry_t entry);
> >>  extern bool reuse_swap_page(struct page *, int *);
> >> diff --git a/mm/swap_state.c b/mm/swap_state.c
> >> index 6c017ced11e6..7dd70e77058d 100644
> >> --- a/mm/swap_state.c
> >> +++ b/mm/swap_state.c
> >> @@ -569,11 +569,13 @@ struct page *swapin_readahead(swp_entry_t entry, 
> >> gfp_t gfp_mask,
> >>/* Read a page_cluster sized and aligned cluster around offset. */
> >>start_offset = offset & ~mask;
> >>end_offset = offset | mask;
> >> -  if (!start_offset)  /* First page is swap header. */
> >> -  start_offset++;
> >>  
> >>blk_start_plug();
> >>for (offset = start_offset; offset <= end_offset ; offset++) {
> >> +  swp_entry_t ent = swp_entry(swp_type(entry), offset);
> >> +
> >> +  if (!swap_entry_check(ent))
> >> +  continue;
> >>/* Ok, do the async read-ahead now */
> >>page = __read_swap_cache_async(
> >>swp_entry(swp_type(entry), offset),
> >> diff --git a/mm/swapfile.c b/mm/swapfile.c
> >> index 3074b02eaa09..b04cec29c234 100644
> >> --- a/mm/swapfile.c
> >> +++ b/mm/swapfile.c
> >> @@ -1107,6 +1107,27 @@ static struct swap_info_struct 
> >> *swap_info_get_cont(swp_entry_t entry,
> >>return p;
> >>  }
> >>  
> >> +bool swap_entry_check(swp_entry_t entry)
> >> +{
> >> +  struct swap_info_struct *p;
> >> +  unsigned long offset, type;
> >> +
> >> +  type = swp_type(entry);
> >> +  if (type >= nr_swapfiles)
> >> +  goto bad_file;
> >> +  p = swap_info[type];
> >> +  offset = swp_offset(entry);
> >> +  if (unlikely(!offset || offset >= p->max))
> >> +  goto out;
> >> +
> >> +  return true;
> >> +
> >> +bad_file:
> >> +  pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
> >> +out:
> >> +  return false;
> >> +}
> >> +
> >>  static unsigned char __swap_entry_free(struct swap_info_struct *p,
> >>   swp_entry_t entry, unsigned char usage)
> >>  {
> >> -- 
> >> 2.14.2
> >
> > Although it's better than old, we can make it simple, still.
> >
> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > index 291c4b534658..f50d5a48f03a 100644
> > --- a/include/linux/swapops.h
> > +++ b/include/linux/swapops.h
> > @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
> > return (entry.val >> SWP_TYPE_SHIFT(entry));
> >  }
> >  
> > +extern struct swap_info_struct *swap_info[];
> > +
> > +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
> > +{
> > +   return swap_info[swp_type(entry)];
> > +}
> > +
> 
> Why not just use swp_swap_info()?

Yeap, I forgot the one that even I added the function. :(

> 
> >  /*
> >   * Extract the `offset' field from a swp_entry_t.  The 

linux-next: Signed-off-by missing for commit in the net-next tree

[Stephen Rothwell]

Hi all,

Commit

  509708310cf9 ("r8169: Add support for interrupt coalesce tuning (ethtool -C)")

is missing a Signed-off-by from its author (or its author is wrong).

-- 
Cheers,
Stephen Rothwell

linux-next: Signed-off-by missing for commit in the net-next tree

[Stephen Rothwell]

Hi all,

Commit

  509708310cf9 ("r8169: Add support for interrupt coalesce tuning (ethtool -C)")

is missing a Signed-off-by from its author (or its author is wrong).

-- 
Cheers,
Stephen Rothwell

[PATCH] MIPS: microMIPS: Fix incorrect mask in insn_table_MM

[Gustavo A. R. Silva]

It seems that this is a typo error and the proper bit masking is
"RT | RS" instead of "RS | RS".

This issue was detected with the help of Coccinelle.

Reported-by: Julia Lawall 
Signed-off-by: Gustavo A. R. Silva 
---
 arch/mips/mm/uasm-micromips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/mm/uasm-micromips.c b/arch/mips/mm/uasm-micromips.c
index c28ff53..cdb5a19 100644
--- a/arch/mips/mm/uasm-micromips.c
+++ b/arch/mips/mm/uasm-micromips.c
@@ -80,7 +80,7 @@ static const struct insn const insn_table_MM[insn_invalid] = {
[insn_jr]   = {M(mm_pool32a_op, 0, 0, 0, mm_jalr_op, 
mm_pool32axf_op), RS},
[insn_lb]   = {M(mm_lb32_op, 0, 0, 0, 0, 0), RT | RS | SIMM},
[insn_ld]   = {0, 0},
-   [insn_lh]   = {M(mm_lh32_op, 0, 0, 0, 0, 0), RS | RS | SIMM},
+   [insn_lh]   = {M(mm_lh32_op, 0, 0, 0, 0, 0), RT | RS | SIMM},
[insn_ll]   = {M(mm_pool32c_op, 0, 0, (mm_ll_func << 1), 0, 0), RS 
| RT | SIMM},
[insn_lld]  = {0, 0},
[insn_lui]  = {M(mm_pool32i_op, mm_lui_op, 0, 0, 0, 0), RS | SIMM},
-- 
2.7.4

[PATCH] MIPS: microMIPS: Fix incorrect mask in insn_table_MM

[Gustavo A. R. Silva]

It seems that this is a typo error and the proper bit masking is
"RT | RS" instead of "RS | RS".

This issue was detected with the help of Coccinelle.

Reported-by: Julia Lawall 
Signed-off-by: Gustavo A. R. Silva 
---
 arch/mips/mm/uasm-micromips.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/mips/mm/uasm-micromips.c b/arch/mips/mm/uasm-micromips.c
index c28ff53..cdb5a19 100644
--- a/arch/mips/mm/uasm-micromips.c
+++ b/arch/mips/mm/uasm-micromips.c
@@ -80,7 +80,7 @@ static const struct insn const insn_table_MM[insn_invalid] = {
[insn_jr]   = {M(mm_pool32a_op, 0, 0, 0, mm_jalr_op, 
mm_pool32axf_op), RS},
[insn_lb]   = {M(mm_lb32_op, 0, 0, 0, 0, 0), RT | RS | SIMM},
[insn_ld]   = {0, 0},
-   [insn_lh]   = {M(mm_lh32_op, 0, 0, 0, 0, 0), RS | RS | SIMM},
+   [insn_lh]   = {M(mm_lh32_op, 0, 0, 0, 0, 0), RT | RS | SIMM},
[insn_ll]   = {M(mm_pool32c_op, 0, 0, (mm_ll_func << 1), 0, 0), RS 
| RT | SIMM},
[insn_lld]  = {0, 0},
[insn_lui]  = {M(mm_pool32i_op, mm_lui_op, 0, 0, 0, 0), RS | SIMM},
-- 
2.7.4

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Huang, Ying]

Hi, Minchan,

Minchan Kim  writes:

> Hi Huang,
>
> On Fri, Oct 27, 2017 at 01:53:27PM +0800, Huang, Ying wrote:
>> From: Huang Ying 
>> 
>> When a page fault occurs for a swap entry, the physical swap readahead
>> (not the VMA base swap readahead) may readahead several swap entries
>> after the fault swap entry.  The readahead algorithm calculates some
>> of the swap entries to readahead via increasing the offset of the
>> fault swap entry without checking whether they are beyond the end of
>> the swap device and it relys on the __swp_swapcount() and
>> swapcache_prepare() to check it.  Although __swp_swapcount() checks
>> for the swap entry passed in, it will complain with the error message
>> as follow for the expected invalid swap entry.  This may make the end
>> users confused.
>> 
>>   swap_info_get: Bad swap offset entry 0200f8a7
>> 
>> To fix the false error message, the swap entry checking is added in
>> swap readahead to avoid to pass the out-bound swap entries and the
>> swap entry reserved for the swap header to __swp_swapcount() and
>> swapcache_prepare().
>> 
>> Cc: Tim Chen 
>> Cc: Minchan Kim 
>> Cc: Michal Hocko 
>> Cc:  # 4.11-4.13
>> Reported-by: Christian Kujau 
>> Fixes: e8c26ab60598 ("mm/swap: skip readahead for unreferenced swap slots")
>> Signed-off-by: "Huang, Ying" 
>> ---
>>  include/linux/swap.h |  1 +
>>  mm/swap_state.c  |  6 --
>>  mm/swapfile.c| 21 +
>>  3 files changed, 26 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index 84255b3da7c1..43b4b821c805 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -476,6 +476,7 @@ extern int page_swapcount(struct page *);
>>  extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
>>  extern int __swp_swapcount(swp_entry_t entry);
>>  extern int swp_swapcount(swp_entry_t entry);
>> +extern bool swap_entry_check(swp_entry_t entry);
>>  extern struct swap_info_struct *page_swap_info(struct page *);
>>  extern struct swap_info_struct *swp_swap_info(swp_entry_t entry);
>>  extern bool reuse_swap_page(struct page *, int *);
>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> index 6c017ced11e6..7dd70e77058d 100644
>> --- a/mm/swap_state.c
>> +++ b/mm/swap_state.c
>> @@ -569,11 +569,13 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
>> gfp_mask,
>>  /* Read a page_cluster sized and aligned cluster around offset. */
>>  start_offset = offset & ~mask;
>>  end_offset = offset | mask;
>> -if (!start_offset)  /* First page is swap header. */
>> -start_offset++;
>>  
>>  blk_start_plug();
>>  for (offset = start_offset; offset <= end_offset ; offset++) {
>> +swp_entry_t ent = swp_entry(swp_type(entry), offset);
>> +
>> +if (!swap_entry_check(ent))
>> +continue;
>>  /* Ok, do the async read-ahead now */
>>  page = __read_swap_cache_async(
>>  swp_entry(swp_type(entry), offset),
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 3074b02eaa09..b04cec29c234 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -1107,6 +1107,27 @@ static struct swap_info_struct 
>> *swap_info_get_cont(swp_entry_t entry,
>>  return p;
>>  }
>>  
>> +bool swap_entry_check(swp_entry_t entry)
>> +{
>> +struct swap_info_struct *p;
>> +unsigned long offset, type;
>> +
>> +type = swp_type(entry);
>> +if (type >= nr_swapfiles)
>> +goto bad_file;
>> +p = swap_info[type];
>> +offset = swp_offset(entry);
>> +if (unlikely(!offset || offset >= p->max))
>> +goto out;
>> +
>> +return true;
>> +
>> +bad_file:
>> +pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
>> +out:
>> +return false;
>> +}
>> +
>>  static unsigned char __swap_entry_free(struct swap_info_struct *p,
>> swp_entry_t entry, unsigned char usage)
>>  {
>> -- 
>> 2.14.2
>
> Although it's better than old, we can make it simple, still.
>
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index 291c4b534658..f50d5a48f03a 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
>   return (entry.val >> SWP_TYPE_SHIFT(entry));
>  }
>  
> +extern struct swap_info_struct *swap_info[];
> +
> +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
> +{
> + return swap_info[swp_type(entry)];
> +}
> +

Why not just use swp_swap_info()?

>  /*
>   * Extract the `offset' field from a swp_entry_t.  The swp_entry_t is in
>   * arch-independent format
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 378262d3a197..a0fe2d54ad09 100644
> --- 

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Huang, Ying]

Hi, Minchan,

Minchan Kim  writes:

> Hi Huang,
>
> On Fri, Oct 27, 2017 at 01:53:27PM +0800, Huang, Ying wrote:
>> From: Huang Ying 
>> 
>> When a page fault occurs for a swap entry, the physical swap readahead
>> (not the VMA base swap readahead) may readahead several swap entries
>> after the fault swap entry.  The readahead algorithm calculates some
>> of the swap entries to readahead via increasing the offset of the
>> fault swap entry without checking whether they are beyond the end of
>> the swap device and it relys on the __swp_swapcount() and
>> swapcache_prepare() to check it.  Although __swp_swapcount() checks
>> for the swap entry passed in, it will complain with the error message
>> as follow for the expected invalid swap entry.  This may make the end
>> users confused.
>> 
>>   swap_info_get: Bad swap offset entry 0200f8a7
>> 
>> To fix the false error message, the swap entry checking is added in
>> swap readahead to avoid to pass the out-bound swap entries and the
>> swap entry reserved for the swap header to __swp_swapcount() and
>> swapcache_prepare().
>> 
>> Cc: Tim Chen 
>> Cc: Minchan Kim 
>> Cc: Michal Hocko 
>> Cc:  # 4.11-4.13
>> Reported-by: Christian Kujau 
>> Fixes: e8c26ab60598 ("mm/swap: skip readahead for unreferenced swap slots")
>> Signed-off-by: "Huang, Ying" 
>> ---
>>  include/linux/swap.h |  1 +
>>  mm/swap_state.c  |  6 --
>>  mm/swapfile.c| 21 +
>>  3 files changed, 26 insertions(+), 2 deletions(-)
>> 
>> diff --git a/include/linux/swap.h b/include/linux/swap.h
>> index 84255b3da7c1..43b4b821c805 100644
>> --- a/include/linux/swap.h
>> +++ b/include/linux/swap.h
>> @@ -476,6 +476,7 @@ extern int page_swapcount(struct page *);
>>  extern int __swap_count(struct swap_info_struct *si, swp_entry_t entry);
>>  extern int __swp_swapcount(swp_entry_t entry);
>>  extern int swp_swapcount(swp_entry_t entry);
>> +extern bool swap_entry_check(swp_entry_t entry);
>>  extern struct swap_info_struct *page_swap_info(struct page *);
>>  extern struct swap_info_struct *swp_swap_info(swp_entry_t entry);
>>  extern bool reuse_swap_page(struct page *, int *);
>> diff --git a/mm/swap_state.c b/mm/swap_state.c
>> index 6c017ced11e6..7dd70e77058d 100644
>> --- a/mm/swap_state.c
>> +++ b/mm/swap_state.c
>> @@ -569,11 +569,13 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
>> gfp_mask,
>>  /* Read a page_cluster sized and aligned cluster around offset. */
>>  start_offset = offset & ~mask;
>>  end_offset = offset | mask;
>> -if (!start_offset)  /* First page is swap header. */
>> -start_offset++;
>>  
>>  blk_start_plug();
>>  for (offset = start_offset; offset <= end_offset ; offset++) {
>> +swp_entry_t ent = swp_entry(swp_type(entry), offset);
>> +
>> +if (!swap_entry_check(ent))
>> +continue;
>>  /* Ok, do the async read-ahead now */
>>  page = __read_swap_cache_async(
>>  swp_entry(swp_type(entry), offset),
>> diff --git a/mm/swapfile.c b/mm/swapfile.c
>> index 3074b02eaa09..b04cec29c234 100644
>> --- a/mm/swapfile.c
>> +++ b/mm/swapfile.c
>> @@ -1107,6 +1107,27 @@ static struct swap_info_struct 
>> *swap_info_get_cont(swp_entry_t entry,
>>  return p;
>>  }
>>  
>> +bool swap_entry_check(swp_entry_t entry)
>> +{
>> +struct swap_info_struct *p;
>> +unsigned long offset, type;
>> +
>> +type = swp_type(entry);
>> +if (type >= nr_swapfiles)
>> +goto bad_file;
>> +p = swap_info[type];
>> +offset = swp_offset(entry);
>> +if (unlikely(!offset || offset >= p->max))
>> +goto out;
>> +
>> +return true;
>> +
>> +bad_file:
>> +pr_err("%s: %s%08lx\n", __func__, Bad_file, entry.val);
>> +out:
>> +return false;
>> +}
>> +
>>  static unsigned char __swap_entry_free(struct swap_info_struct *p,
>> swp_entry_t entry, unsigned char usage)
>>  {
>> -- 
>> 2.14.2
>
> Although it's better than old, we can make it simple, still.
>
> diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> index 291c4b534658..f50d5a48f03a 100644
> --- a/include/linux/swapops.h
> +++ b/include/linux/swapops.h
> @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
>   return (entry.val >> SWP_TYPE_SHIFT(entry));
>  }
>  
> +extern struct swap_info_struct *swap_info[];
> +
> +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
> +{
> + return swap_info[swp_type(entry)];
> +}
> +

Why not just use swp_swap_info()?

>  /*
>   * Extract the `offset' field from a swp_entry_t.  The swp_entry_t is in
>   * arch-independent format
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index 378262d3a197..a0fe2d54ad09 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -554,6 +554,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
> gfp_mask,
>   struct vm_area_struct *vma, unsigned long 

[PATCH RFC 0/2] Fix race window during idle cpu selection.

[Atish Patra]

The 1st patch actually fixes the issue. The 2nd patch adds a new element
in schedstat intended only for testing.

Atish Patra (2):
  sched: Minimize the idle cpu selection race window.
  sched: Add a stat for idle cpu selection race window.

 kernel/sched/core.c  |  6 ++
 kernel/sched/fair.c  | 12 +---
 kernel/sched/idle_task.c |  1 +
 kernel/sched/sched.h |  2 ++
 kernel/sched/stats.c |  5 +++--
 5 files changed, 21 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH RFC 0/2] Fix race window during idle cpu selection.

[Atish Patra]

The 1st patch actually fixes the issue. The 2nd patch adds a new element
in schedstat intended only for testing.

Atish Patra (2):
  sched: Minimize the idle cpu selection race window.
  sched: Add a stat for idle cpu selection race window.

 kernel/sched/core.c  |  6 ++
 kernel/sched/fair.c  | 12 +---
 kernel/sched/idle_task.c |  1 +
 kernel/sched/sched.h |  2 ++
 kernel/sched/stats.c |  5 +++--
 5 files changed, 21 insertions(+), 5 deletions(-)

-- 
2.7.4

[PATCH DEBUG 2/2] sched: Add a stat for idle cpu selection race window.

[Atish Patra]

This is ** Debug ** only patch not intended for merging.

A new stat in schedstat is added that represents number of
times cpu was already claimed during wakeup while some other
cpu tries to schedule tasks on it again. It helps to verify
if the concerned issue is present in a specific becnhmark.

Tested-by: Joel Fernandes 
Signed-off-by: Atish Patra 
---
 kernel/sched/core.c  | 4 +++-
 kernel/sched/sched.h | 1 +
 kernel/sched/stats.c | 5 +++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d9d501c..d12b8c8 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3918,8 +3918,10 @@ int idle_cpu(int cpu)
return 0;
 #endif
 
-   if (per_cpu(claim_wakeup, cpu))
+   if (per_cpu(claim_wakeup, cpu)) {
+   schedstat_inc(rq->ttwu_claimed);
return 0;
+   }
 
return 1;
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 5f70b98..8f3047c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -801,6 +801,7 @@ struct rq {
/* try_to_wake_up() stats */
unsigned int ttwu_count;
unsigned int ttwu_local;
+   unsigned int ttwu_claimed;
 #endif
 
 #ifdef CONFIG_SMP
diff --git a/kernel/sched/stats.c b/kernel/sched/stats.c
index 87e2c9f..2290aa7 100644
--- a/kernel/sched/stats.c
+++ b/kernel/sched/stats.c
@@ -30,12 +30,13 @@ static int show_schedstat(struct seq_file *seq, void *v)
 
/* runqueue-specific stats */
seq_printf(seq,
-   "cpu%d %u 0 %u %u %u %u %llu %llu %lu",
+   "cpu%d %u 0 %u %u %u %u %llu %llu %lu %u",
cpu, rq->yld_count,
rq->sched_count, rq->sched_goidle,
rq->ttwu_count, rq->ttwu_local,
rq->rq_cpu_time,
-   rq->rq_sched_info.run_delay, rq->rq_sched_info.pcount);
+   rq->rq_sched_info.run_delay,
+   rq->rq_sched_info.pcount, rq->ttwu_claimed);
 
seq_printf(seq, "\n");
 
-- 
2.7.4

[PATCH DEBUG 2/2] sched: Add a stat for idle cpu selection race window.

[Atish Patra]

This is ** Debug ** only patch not intended for merging.

A new stat in schedstat is added that represents number of
times cpu was already claimed during wakeup while some other
cpu tries to schedule tasks on it again. It helps to verify
if the concerned issue is present in a specific becnhmark.

Tested-by: Joel Fernandes 
Signed-off-by: Atish Patra 
---
 kernel/sched/core.c  | 4 +++-
 kernel/sched/sched.h | 1 +
 kernel/sched/stats.c | 5 +++--
 3 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d9d501c..d12b8c8 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3918,8 +3918,10 @@ int idle_cpu(int cpu)
return 0;
 #endif
 
-   if (per_cpu(claim_wakeup, cpu))
+   if (per_cpu(claim_wakeup, cpu)) {
+   schedstat_inc(rq->ttwu_claimed);
return 0;
+   }
 
return 1;
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 5f70b98..8f3047c 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -801,6 +801,7 @@ struct rq {
/* try_to_wake_up() stats */
unsigned int ttwu_count;
unsigned int ttwu_local;
+   unsigned int ttwu_claimed;
 #endif
 
 #ifdef CONFIG_SMP
diff --git a/kernel/sched/stats.c b/kernel/sched/stats.c
index 87e2c9f..2290aa7 100644
--- a/kernel/sched/stats.c
+++ b/kernel/sched/stats.c
@@ -30,12 +30,13 @@ static int show_schedstat(struct seq_file *seq, void *v)
 
/* runqueue-specific stats */
seq_printf(seq,
-   "cpu%d %u 0 %u %u %u %u %llu %llu %lu",
+   "cpu%d %u 0 %u %u %u %u %llu %llu %lu %u",
cpu, rq->yld_count,
rq->sched_count, rq->sched_goidle,
rq->ttwu_count, rq->ttwu_local,
rq->rq_cpu_time,
-   rq->rq_sched_info.run_delay, rq->rq_sched_info.pcount);
+   rq->rq_sched_info.run_delay,
+   rq->rq_sched_info.pcount, rq->ttwu_claimed);
 
seq_printf(seq, "\n");
 
-- 
2.7.4

[PATCH RFC 1/2] sched: Minimize the idle cpu selection race window.

[Atish Patra]

Currently, multiple tasks can wakeup on same cpu from
select_idle_sibiling() path in case they wakeup simulatenously
and last ran on the same llc. This happens because an idle cpu
is not updated until idle task is scheduled out. Any task waking
during that period may potentially select that cpu for a wakeup
candidate.

Introduce a per cpu variable that is set as soon as a cpu is
selected for wakeup for any task. This prevents from other tasks
to select the same cpu again. Note: This does not close the race
window but minimizes it to accessing the per-cpu variable. If two
wakee tasks access the per cpu variable at the same time, they may
select the same cpu again. But it minimizes the race window
considerably.

Suggested-by: Peter Zijlstra 
Tested-by: Joel Fernandes 
Signed-off-by: Atish Patra 
---
 kernel/sched/core.c  |  4 
 kernel/sched/fair.c  | 12 +---
 kernel/sched/idle_task.c |  1 +
 kernel/sched/sched.h |  1 +
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2288a14..d9d501c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3896,6 +3896,7 @@ int task_prio(const struct task_struct *p)
return p->prio - MAX_RT_PRIO;
 }
 
+DEFINE_PER_CPU(int, claim_wakeup);
 /**
  * idle_cpu - is a given CPU idle currently?
  * @cpu: the processor in question.
@@ -3917,6 +3918,9 @@ int idle_cpu(int cpu)
return 0;
 #endif
 
+   if (per_cpu(claim_wakeup, cpu))
+   return 0;
+
return 1;
 }
 
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 13393bb..885023a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6077,8 +6077,10 @@ static int select_idle_core(struct task_struct *p, 
struct sched_domain *sd, int
idle = false;
}
 
-   if (idle)
+   if (idle) {
+   per_cpu(claim_wakeup, core) = 1;
return core;
+   }
}
 
/*
@@ -6102,8 +6104,10 @@ static int select_idle_smt(struct task_struct *p, struct 
sched_domain *sd, int t
for_each_cpu(cpu, cpu_smt_mask(target)) {
if (!cpumask_test_cpu(cpu, >cpus_allowed))
continue;
-   if (idle_cpu(cpu))
+   if (idle_cpu(cpu)) {
+   per_cpu(claim_wakeup, cpu) = 1;
return cpu;
+   }
}
 
return -1;
@@ -6165,8 +6169,10 @@ static int select_idle_cpu(struct task_struct *p, struct 
sched_domain *sd, int t
return -1;
if (!cpumask_test_cpu(cpu, >cpus_allowed))
continue;
-   if (idle_cpu(cpu))
+   if (idle_cpu(cpu)) {
+   per_cpu(claim_wakeup, cpu) = 1;
break;
+   }
}
 
time = local_clock() - time;
diff --git a/kernel/sched/idle_task.c b/kernel/sched/idle_task.c
index 0c00172..64d6495 100644
--- a/kernel/sched/idle_task.c
+++ b/kernel/sched/idle_task.c
@@ -28,6 +28,7 @@ pick_next_task_idle(struct rq *rq, struct task_struct *prev, 
struct rq_flags *rf
 {
put_prev_task(rq, prev);
update_idle_core(rq);
+   this_cpu_write(claim_wakeup, 0);
schedstat_inc(rq->sched_goidle);
return rq->idle;
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 8aa24b4..5f70b98 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1068,6 +1068,7 @@ DECLARE_PER_CPU(int, sd_llc_id);
 DECLARE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
 DECLARE_PER_CPU(struct sched_domain *, sd_numa);
 DECLARE_PER_CPU(struct sched_domain *, sd_asym);
+DECLARE_PER_CPU(int, claim_wakeup);
 
 struct sched_group_capacity {
atomic_t ref;
-- 
2.7.4

[PATCH RFC 1/2] sched: Minimize the idle cpu selection race window.

[Atish Patra]

Currently, multiple tasks can wakeup on same cpu from
select_idle_sibiling() path in case they wakeup simulatenously
and last ran on the same llc. This happens because an idle cpu
is not updated until idle task is scheduled out. Any task waking
during that period may potentially select that cpu for a wakeup
candidate.

Introduce a per cpu variable that is set as soon as a cpu is
selected for wakeup for any task. This prevents from other tasks
to select the same cpu again. Note: This does not close the race
window but minimizes it to accessing the per-cpu variable. If two
wakee tasks access the per cpu variable at the same time, they may
select the same cpu again. But it minimizes the race window
considerably.

Suggested-by: Peter Zijlstra 
Tested-by: Joel Fernandes 
Signed-off-by: Atish Patra 
---
 kernel/sched/core.c  |  4 
 kernel/sched/fair.c  | 12 +---
 kernel/sched/idle_task.c |  1 +
 kernel/sched/sched.h |  1 +
 4 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 2288a14..d9d501c 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -3896,6 +3896,7 @@ int task_prio(const struct task_struct *p)
return p->prio - MAX_RT_PRIO;
 }
 
+DEFINE_PER_CPU(int, claim_wakeup);
 /**
  * idle_cpu - is a given CPU idle currently?
  * @cpu: the processor in question.
@@ -3917,6 +3918,9 @@ int idle_cpu(int cpu)
return 0;
 #endif
 
+   if (per_cpu(claim_wakeup, cpu))
+   return 0;
+
return 1;
 }
 
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index 13393bb..885023a 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -6077,8 +6077,10 @@ static int select_idle_core(struct task_struct *p, 
struct sched_domain *sd, int
idle = false;
}
 
-   if (idle)
+   if (idle) {
+   per_cpu(claim_wakeup, core) = 1;
return core;
+   }
}
 
/*
@@ -6102,8 +6104,10 @@ static int select_idle_smt(struct task_struct *p, struct 
sched_domain *sd, int t
for_each_cpu(cpu, cpu_smt_mask(target)) {
if (!cpumask_test_cpu(cpu, >cpus_allowed))
continue;
-   if (idle_cpu(cpu))
+   if (idle_cpu(cpu)) {
+   per_cpu(claim_wakeup, cpu) = 1;
return cpu;
+   }
}
 
return -1;
@@ -6165,8 +6169,10 @@ static int select_idle_cpu(struct task_struct *p, struct 
sched_domain *sd, int t
return -1;
if (!cpumask_test_cpu(cpu, >cpus_allowed))
continue;
-   if (idle_cpu(cpu))
+   if (idle_cpu(cpu)) {
+   per_cpu(claim_wakeup, cpu) = 1;
break;
+   }
}
 
time = local_clock() - time;
diff --git a/kernel/sched/idle_task.c b/kernel/sched/idle_task.c
index 0c00172..64d6495 100644
--- a/kernel/sched/idle_task.c
+++ b/kernel/sched/idle_task.c
@@ -28,6 +28,7 @@ pick_next_task_idle(struct rq *rq, struct task_struct *prev, 
struct rq_flags *rf
 {
put_prev_task(rq, prev);
update_idle_core(rq);
+   this_cpu_write(claim_wakeup, 0);
schedstat_inc(rq->sched_goidle);
return rq->idle;
 }
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index 8aa24b4..5f70b98 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1068,6 +1068,7 @@ DECLARE_PER_CPU(int, sd_llc_id);
 DECLARE_PER_CPU(struct sched_domain_shared *, sd_llc_shared);
 DECLARE_PER_CPU(struct sched_domain *, sd_numa);
 DECLARE_PER_CPU(struct sched_domain *, sd_asym);
+DECLARE_PER_CPU(int, claim_wakeup);
 
 struct sched_group_capacity {
atomic_t ref;
-- 
2.7.4

Re: [PATCH] USB: serial: io_edgeport: mark expected switch fall-throughs

[Gustavo A. R. Silva]

Hi David, Johan,

Quoting Johan Hovold :


On Mon, Oct 30, 2017 at 11:54:33AM +, David Laight wrote:

From: Bjørn Mork
> Sent: 28 October 2017 11:57
> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > where we are expecting to fall through.
> >
> > Notice that in this particular case I replaced "...drop on through"
> > comments with a proper "fall through" comment on its own line, which
> > is what GCC is expecting to find.
>
> Sounds to me like GCC is the wrong tool for this.  But I would of course
> not mind if it was *just* the text.  However, as your patch cleary
> shows, the simplified logic leads to real problems:
>
> > @@ -1819,8 +1819,8 @@ static void process_rcvd_data(struct  
edgeport_serial *edge_serial,

> >   edge_serial->rxState = EXPECT_DATA;
> >   break;
> >   }
> > - /* Else, drop through */
> >   }
> > + /* fall through */
> >   case EXPECT_DATA: /* Expect data */
> >   if (bufferLength < edge_serial->rxBytesRemaining) {
> >   rxLen = bufferLength;
>
>
> The original comment clearly marked a *conditional* fall through at the
> correct place.  Your patch makes it appear as if there is an
> unconditional fall through here.  There is not.  The fallthrough only
> applies to one of a number of nested if blocks. There are no less than
> 3 break statements in the same case block.
>
> Not a big deal maybe, just as the lack of any "fall through" comment
> isn't a big deal in the first place.  But this change is clearly making
> this code harder to read, and the change is therefore harmful IMHO.
>
> If you can't make -Wimplicit-fallthrough work without removing such
> precise fallthrough markings, then my proposal is to drop it and use
> some other tool.

Just remove the 'else' after the 'break' further up.


Yeah, that might be a good way to resolve this. I was gonna suggest
adding the "fall though" comment before the case while keeping the
"Else, drop through" comment in the branch, but removing the else might
be better.



Thanks for the suggestion.


This code is pretty hard to read as is and could really use some clean
up...



I agree. I'll send a V2 of this patch and then let's see if I can help  
with some code refactoring.


Thank you
--
Gustavo A. R. Silva

Re: [PATCH] USB: serial: io_edgeport: mark expected switch fall-throughs

[Gustavo A. R. Silva]

Hi David, Johan,

Quoting Johan Hovold :


On Mon, Oct 30, 2017 at 11:54:33AM +, David Laight wrote:

From: Bjørn Mork
> Sent: 28 October 2017 11:57
> > In preparation to enabling -Wimplicit-fallthrough, mark switch cases
> > where we are expecting to fall through.
> >
> > Notice that in this particular case I replaced "...drop on through"
> > comments with a proper "fall through" comment on its own line, which
> > is what GCC is expecting to find.
>
> Sounds to me like GCC is the wrong tool for this.  But I would of course
> not mind if it was *just* the text.  However, as your patch cleary
> shows, the simplified logic leads to real problems:
>
> > @@ -1819,8 +1819,8 @@ static void process_rcvd_data(struct  
edgeport_serial *edge_serial,

> >   edge_serial->rxState = EXPECT_DATA;
> >   break;
> >   }
> > - /* Else, drop through */
> >   }
> > + /* fall through */
> >   case EXPECT_DATA: /* Expect data */
> >   if (bufferLength < edge_serial->rxBytesRemaining) {
> >   rxLen = bufferLength;
>
>
> The original comment clearly marked a *conditional* fall through at the
> correct place.  Your patch makes it appear as if there is an
> unconditional fall through here.  There is not.  The fallthrough only
> applies to one of a number of nested if blocks. There are no less than
> 3 break statements in the same case block.
>
> Not a big deal maybe, just as the lack of any "fall through" comment
> isn't a big deal in the first place.  But this change is clearly making
> this code harder to read, and the change is therefore harmful IMHO.
>
> If you can't make -Wimplicit-fallthrough work without removing such
> precise fallthrough markings, then my proposal is to drop it and use
> some other tool.

Just remove the 'else' after the 'break' further up.


Yeah, that might be a good way to resolve this. I was gonna suggest
adding the "fall though" comment before the case while keeping the
"Else, drop through" comment in the branch, but removing the else might
be better.



Thanks for the suggestion.


This code is pretty hard to read as is and could really use some clean
up...



I agree. I'll send a V2 of this patch and then let's see if I can help  
with some code refactoring.


Thank you
--
Gustavo A. R. Silva

Re: [PATCH v2 05/17] PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This way pci-dra7xx.c does not need its own copy of dw_pcie_ep_reset_bar().
> 
> Signed-off-by: Niklas Cassel 

Acked-by: Kishon Vijay Abraham I 
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c | 9 -
>  drivers/pci/dwc/pcie-designware-ep.c | 2 +-
>  drivers/pci/dwc/pcie-designware.h| 5 +
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index d0848006945a..a743545da4d4 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -336,15 +336,6 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void 
> *arg)
>   return IRQ_HANDLED;
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> -{
> - u32 reg;
> -
> - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> - dw_pcie_writel_dbi2(pci, reg, 0x0);
> - dw_pcie_writel_dbi(pci, reg, 0x0);
> -}
> -
>  static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> b/drivers/pci/dwc/pcie-designware-ep.c
> index c291da2a10ba..47134a85a342 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -30,7 +30,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>   pci_epc_linkup(epc);
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
>   u32 reg;
>  
> diff --git a/drivers/pci/dwc/pcie-designware.h 
> b/drivers/pci/dwc/pcie-designware.h
> index 5a1da459eda5..37dfad8d003f 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -338,6 +338,7 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
>  void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>  int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
>  #else
>  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  {
> @@ -351,5 +352,9 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  {
>  }
> +
> +static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno 
> bar)
> +{
> +}
>  #endif
>  #endif /* _PCIE_DESIGNWARE_H */
> 

Re: [PATCH v2 05/17] PCI: designware-ep: Remove static keyword from dw_pcie_ep_reset_bar()

[Kishon Vijay Abraham I]

On Monday 30 October 2017 06:12 PM, Niklas Cassel wrote:
> This way pci-dra7xx.c does not need its own copy of dw_pcie_ep_reset_bar().
> 
> Signed-off-by: Niklas Cassel 

Acked-by: Kishon Vijay Abraham I 
> ---
> V2:
> * New patch in this series.
> 
>  drivers/pci/dwc/pci-dra7xx.c | 9 -
>  drivers/pci/dwc/pcie-designware-ep.c | 2 +-
>  drivers/pci/dwc/pcie-designware.h| 5 +
>  3 files changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/dwc/pci-dra7xx.c b/drivers/pci/dwc/pci-dra7xx.c
> index d0848006945a..a743545da4d4 100644
> --- a/drivers/pci/dwc/pci-dra7xx.c
> +++ b/drivers/pci/dwc/pci-dra7xx.c
> @@ -336,15 +336,6 @@ static irqreturn_t dra7xx_pcie_irq_handler(int irq, void 
> *arg)
>   return IRQ_HANDLED;
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> -{
> - u32 reg;
> -
> - reg = PCI_BASE_ADDRESS_0 + (4 * bar);
> - dw_pcie_writel_dbi2(pci, reg, 0x0);
> - dw_pcie_writel_dbi(pci, reg, 0x0);
> -}
> -
>  static void dra7xx_pcie_ep_init(struct dw_pcie_ep *ep)
>  {
>   struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> diff --git a/drivers/pci/dwc/pcie-designware-ep.c 
> b/drivers/pci/dwc/pcie-designware-ep.c
> index c291da2a10ba..47134a85a342 100644
> --- a/drivers/pci/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/dwc/pcie-designware-ep.c
> @@ -30,7 +30,7 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>   pci_epc_linkup(epc);
>  }
>  
> -static void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
>  {
>   u32 reg;
>  
> diff --git a/drivers/pci/dwc/pcie-designware.h 
> b/drivers/pci/dwc/pcie-designware.h
> index 5a1da459eda5..37dfad8d003f 100644
> --- a/drivers/pci/dwc/pcie-designware.h
> +++ b/drivers/pci/dwc/pcie-designware.h
> @@ -338,6 +338,7 @@ static inline int dw_pcie_host_init(struct pcie_port *pp)
>  void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
>  int dw_pcie_ep_init(struct dw_pcie_ep *ep);
>  void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
> +void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
>  #else
>  static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
>  {
> @@ -351,5 +352,9 @@ static inline int dw_pcie_ep_init(struct dw_pcie_ep *ep)
>  static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
>  {
>  }
> +
> +static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno 
> bar)
> +{
> +}
>  #endif
>  #endif /* _PCIE_DESIGNWARE_H */
> 

Re: Kernel crash in free_pipe_info()

[Al Viro]

On Mon, Oct 30, 2017 at 08:06:23PM -0700, Linus Torvalds wrote:

> We do that "free_pipe_info(inode->i_pipe);", but we never actually
> clear inode->i_pipe, so now we have an inode that looks like a pipe
> inode, and has a stale pointer to a pipe_inode_info.
> 
> It all looks technically correct. It's fine to use put_filp(), because
> the file pointer has never really been used. And the inode should
> never get re-used anyway without going through the whole reinit in
> inode_init_always().
> 
> So I don't see anything *wrong*, but I see a lot that is just unusual,

FWIW, it's really brittle - consider
if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
i_readcount_inc(path->dentry->d_inode);
in alloc_file().  It's not the source of trouble in this case, but only
because it's the second call that gets FMODE_READ; reorder them in
create_pipe_files() and you've got a bug.

I considered using fput() there, but that would've required manually
decrementing pipe->files first, which made it rather unappealing...

I don't see anything relevant there, but that's not saying much - flu
and debugging do not mix well, and lack of sleep also doesn't help ;-/

Re: Kernel crash in free_pipe_info()

[Al Viro]

On Mon, Oct 30, 2017 at 08:06:23PM -0700, Linus Torvalds wrote:

> We do that "free_pipe_info(inode->i_pipe);", but we never actually
> clear inode->i_pipe, so now we have an inode that looks like a pipe
> inode, and has a stale pointer to a pipe_inode_info.
> 
> It all looks technically correct. It's fine to use put_filp(), because
> the file pointer has never really been used. And the inode should
> never get re-used anyway without going through the whole reinit in
> inode_init_always().
> 
> So I don't see anything *wrong*, but I see a lot that is just unusual,

FWIW, it's really brittle - consider
if ((mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
i_readcount_inc(path->dentry->d_inode);
in alloc_file().  It's not the source of trouble in this case, but only
because it's the second call that gets FMODE_READ; reorder them in
create_pipe_files() and you've got a bug.

I considered using fput() there, but that would've required manually
decrementing pipe->files first, which made it rather unappealing...

I don't see anything relevant there, but that's not saying much - flu
and debugging do not mix well, and lack of sleep also doesn't help ;-/

[PATCH] x86, build: Improve the isolinux searching of isoimage generation

[changbin . du]

From: Changbin Du 

Recently I failed to build isoimage target, because the path of isolinux.bin
changed to /usr/xxx/ISOLINUX/isolinux.bin, as well as ldlinux.c32 which
changed to /usr/xxx/syslinux/modules/bios/ldlinux.c32.

This patch has a improvement of the file search:
  - Don't print the raw shell commands. It doesn't make sense to show the
entire big block.
  - Show a error message instead of silent fail.
  - Add above new paths.

Now it becomes:
Kernel: arch/x86/boot/bzImage is ready  (#62)
rm -rf arch/x86/boot/isoimage
mkdir arch/x86/boot/isoimage
Using /usr/lib/ISOLINUX/isolinux.bin
Using /usr/lib/syslinux/modules/bios/ldlinux.c32
cp arch/x86/boot/bzImage arch/x86/boot/isoimage/linux
...

Before:
Kernel: arch/x86/boot/bzImage is ready  (#63)
rm -rf arch/x86/boot/isoimage
mkdir arch/x86/boot/isoimage
for i in lib lib64 share end ; do \
if [ -f /usr/$i/syslinux/isolinux.bin ] ; then \
cp /usr/$i/syslinux/isolinux.bin arch/x86/boot/isoimage ; \
if [ -f /usr/$i/syslinux/ldlinux.c32 ]; then \
cp /usr/$i/syslinux/ldlinux.c32 arch/x86/boot/isoimage 
; \
fi ; \
break ; \
fi ; \
if [ $i = end ] ; then exit 1 ; fi ; \
done
arch/x86/boot/Makefile:161: recipe for target 'isoimage' failed
make[1]: *** [isoimage] Error 1

Signed-off-by: Changbin Du 
---
 arch/x86/boot/Makefile | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index d88a2fd..8425c2f 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -160,15 +160,28 @@ fdimage288: $(obj)/bzImage $(obj)/mtools.conf
 isoimage: $(obj)/bzImage
-rm -rf $(obj)/isoimage
mkdir $(obj)/isoimage
-   for i in lib lib64 share end ; do \
-   if [ -f /usr/$$i/syslinux/isolinux.bin ] ; then \
-   cp /usr/$$i/syslinux/isolinux.bin $(obj)/isoimage ; \
-   if [ -f /usr/$$i/syslinux/ldlinux.c32 ]; then \
-   cp /usr/$$i/syslinux/ldlinux.c32 
$(obj)/isoimage ; \
+   @for i in lib lib64 share end ; do \
+   for j in syslinux ISOLINUX ; do \
+   if [ -f /usr/$$i/$$j/isolinux.bin ] ; then \
+   isolinux=/usr/$$i/$$j/isolinux.bin ; \
+   echo "Using $$isolinux" ; \
+   cp $$isolinux $(obj)/isoimage ; \
fi ; \
+   done ; \
+   for j in syslinux syslinux/modules/bios ; do \
+   if [ -f /usr/$$i/$$j/ldlinux.c32 ]; then \
+   ldlinux=/usr/$$i/$$j/ldlinux.c32 ; \
+   echo "Using $$ldlinux" ; \
+   cp $$ldlinux $(obj)/isoimage ; \
+   fi ; \
+   done ; \
+   if [ -n "$$isolinux" -a -n "$$ldlinux" ] ; then \
break ; \
fi ; \
-   if [ $$i = end ] ; then exit 1 ; fi ; \
+   if [ $$i = end -a -z "$$isolinux" ] ; then \
+   echo 'Need isolinux.bin, please install 
syslinux/isolinux' ; \
+   exit 1 ; \
+   fi ; \
done
cp $(obj)/bzImage $(obj)/isoimage/linux
echo '$(image_cmdline)' > $(obj)/isoimage/isolinux.cfg
-- 
2.7.4

[PATCH] x86, build: Improve the isolinux searching of isoimage generation

[changbin . du]

From: Changbin Du 

Recently I failed to build isoimage target, because the path of isolinux.bin
changed to /usr/xxx/ISOLINUX/isolinux.bin, as well as ldlinux.c32 which
changed to /usr/xxx/syslinux/modules/bios/ldlinux.c32.

This patch has a improvement of the file search:
  - Don't print the raw shell commands. It doesn't make sense to show the
entire big block.
  - Show a error message instead of silent fail.
  - Add above new paths.

Now it becomes:
Kernel: arch/x86/boot/bzImage is ready  (#62)
rm -rf arch/x86/boot/isoimage
mkdir arch/x86/boot/isoimage
Using /usr/lib/ISOLINUX/isolinux.bin
Using /usr/lib/syslinux/modules/bios/ldlinux.c32
cp arch/x86/boot/bzImage arch/x86/boot/isoimage/linux
...

Before:
Kernel: arch/x86/boot/bzImage is ready  (#63)
rm -rf arch/x86/boot/isoimage
mkdir arch/x86/boot/isoimage
for i in lib lib64 share end ; do \
if [ -f /usr/$i/syslinux/isolinux.bin ] ; then \
cp /usr/$i/syslinux/isolinux.bin arch/x86/boot/isoimage ; \
if [ -f /usr/$i/syslinux/ldlinux.c32 ]; then \
cp /usr/$i/syslinux/ldlinux.c32 arch/x86/boot/isoimage 
; \
fi ; \
break ; \
fi ; \
if [ $i = end ] ; then exit 1 ; fi ; \
done
arch/x86/boot/Makefile:161: recipe for target 'isoimage' failed
make[1]: *** [isoimage] Error 1

Signed-off-by: Changbin Du 
---
 arch/x86/boot/Makefile | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index d88a2fd..8425c2f 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -160,15 +160,28 @@ fdimage288: $(obj)/bzImage $(obj)/mtools.conf
 isoimage: $(obj)/bzImage
-rm -rf $(obj)/isoimage
mkdir $(obj)/isoimage
-   for i in lib lib64 share end ; do \
-   if [ -f /usr/$$i/syslinux/isolinux.bin ] ; then \
-   cp /usr/$$i/syslinux/isolinux.bin $(obj)/isoimage ; \
-   if [ -f /usr/$$i/syslinux/ldlinux.c32 ]; then \
-   cp /usr/$$i/syslinux/ldlinux.c32 
$(obj)/isoimage ; \
+   @for i in lib lib64 share end ; do \
+   for j in syslinux ISOLINUX ; do \
+   if [ -f /usr/$$i/$$j/isolinux.bin ] ; then \
+   isolinux=/usr/$$i/$$j/isolinux.bin ; \
+   echo "Using $$isolinux" ; \
+   cp $$isolinux $(obj)/isoimage ; \
fi ; \
+   done ; \
+   for j in syslinux syslinux/modules/bios ; do \
+   if [ -f /usr/$$i/$$j/ldlinux.c32 ]; then \
+   ldlinux=/usr/$$i/$$j/ldlinux.c32 ; \
+   echo "Using $$ldlinux" ; \
+   cp $$ldlinux $(obj)/isoimage ; \
+   fi ; \
+   done ; \
+   if [ -n "$$isolinux" -a -n "$$ldlinux" ] ; then \
break ; \
fi ; \
-   if [ $$i = end ] ; then exit 1 ; fi ; \
+   if [ $$i = end -a -z "$$isolinux" ] ; then \
+   echo 'Need isolinux.bin, please install 
syslinux/isolinux' ; \
+   exit 1 ; \
+   fi ; \
done
cp $(obj)/bzImage $(obj)/isoimage/linux
echo '$(image_cmdline)' > $(obj)/isoimage/isolinux.cfg
-- 
2.7.4

[PATCH v2] staging: rtlwifi: Fix line too long warning

[Kien Ha]

>From aa0f4ae8c325545b1fd794d6bbf8c4d2f64e2ec2 Mon Sep 17 00:00:00 2001
From: Kien Ha 
Date: Fri, 27 Oct 2017 14:07:55 -0400
Subject: [PATCH v2] staging: rtlwifi: Fix line too long warning

Made nested if else statement more concise to help conform to coding
style.

Signed-off-by: Kien Ha 
---
Changes in v2:
- Improve block of code to be more concise

 drivers/staging/rtlwifi/base.c | 25 -
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
index b88b0e8edd3d..fdd1ab1e38c5 100644
--- a/drivers/staging/rtlwifi/base.c
+++ b/drivers/staging/rtlwifi/base.c
@@ -1273,23 +1273,14 @@ void rtl_get_tcb_desc(struct ieee80211_hw *hw,
 * and N rate will all be controlled by FW
 * when tcb_desc->use_driver_rate = false
 */
-   if (sta && sta->vht_cap.vht_supported) {
-   tcb_desc->hw_rate =
-   _rtl_get_vht_highest_n_rate(hw, sta);
-   } else {
-   if (sta && (sta->ht_cap.ht_supported)) {
-   tcb_desc->hw_rate =
-   _rtl_get_highest_n_rate(hw, sta);
-   } else {
-   if (rtlmac->mode == WIRELESS_MODE_B) {
-   tcb_desc->hw_rate =
-   
rtlpriv->cfg->maps[RTL_RC_CCK_RATE11M];
-   } else {
-   tcb_desc->hw_rate =
-   
rtlpriv->cfg->maps[RTL_RC_OFDM_RATE54M];
-   }
-   }
-   }
+   tcb_desc->hw_rate =
+   sta && sta->vht_cap.vht_supported ?
+   rtl_get_vht_highest_n_rate(hw, sta) :
+   sta && sta->ht_cap.ht_supported ?
+   _rtl_get_highest_n_rate(hw, sta) :
+   rtlmac->mode == WIRELESS_MODE_B ?
+   rtlpriv->cfg->maps[RTL_RC_CCK_RATE11M] :
+   rtlpriv->cfg->maps[RTL_RC_OFDM_RATE54M];
}
 
if (is_multicast_ether_addr(hdr->addr1))
-- 
2.14.3

[PATCH v2] staging: rtlwifi: Fix line too long warning

[Kien Ha]

>From aa0f4ae8c325545b1fd794d6bbf8c4d2f64e2ec2 Mon Sep 17 00:00:00 2001
From: Kien Ha 
Date: Fri, 27 Oct 2017 14:07:55 -0400
Subject: [PATCH v2] staging: rtlwifi: Fix line too long warning

Made nested if else statement more concise to help conform to coding
style.

Signed-off-by: Kien Ha 
---
Changes in v2:
- Improve block of code to be more concise

 drivers/staging/rtlwifi/base.c | 25 -
 1 file changed, 8 insertions(+), 17 deletions(-)

diff --git a/drivers/staging/rtlwifi/base.c b/drivers/staging/rtlwifi/base.c
index b88b0e8edd3d..fdd1ab1e38c5 100644
--- a/drivers/staging/rtlwifi/base.c
+++ b/drivers/staging/rtlwifi/base.c
@@ -1273,23 +1273,14 @@ void rtl_get_tcb_desc(struct ieee80211_hw *hw,
 * and N rate will all be controlled by FW
 * when tcb_desc->use_driver_rate = false
 */
-   if (sta && sta->vht_cap.vht_supported) {
-   tcb_desc->hw_rate =
-   _rtl_get_vht_highest_n_rate(hw, sta);
-   } else {
-   if (sta && (sta->ht_cap.ht_supported)) {
-   tcb_desc->hw_rate =
-   _rtl_get_highest_n_rate(hw, sta);
-   } else {
-   if (rtlmac->mode == WIRELESS_MODE_B) {
-   tcb_desc->hw_rate =
-   
rtlpriv->cfg->maps[RTL_RC_CCK_RATE11M];
-   } else {
-   tcb_desc->hw_rate =
-   
rtlpriv->cfg->maps[RTL_RC_OFDM_RATE54M];
-   }
-   }
-   }
+   tcb_desc->hw_rate =
+   sta && sta->vht_cap.vht_supported ?
+   rtl_get_vht_highest_n_rate(hw, sta) :
+   sta && sta->ht_cap.ht_supported ?
+   _rtl_get_highest_n_rate(hw, sta) :
+   rtlmac->mode == WIRELESS_MODE_B ?
+   rtlpriv->cfg->maps[RTL_RC_CCK_RATE11M] :
+   rtlpriv->cfg->maps[RTL_RC_OFDM_RATE54M];
}
 
if (is_multicast_ether_addr(hdr->addr1))
-- 
2.14.3

Re: Kernel crash in free_pipe_info()

[Al Viro]

On Mon, Oct 30, 2017 at 07:08:46PM -0700, Linus Torvalds wrote:
> On Mon, Oct 30, 2017 at 6:19 PM, Cong Wang  wrote:
> >
> > 1. The faulty addresses are all near 0001, with one exception
> > of null (which is the most recent one)
> 
> Well, they're at 8(%rax), except for that last case.

0x10(%rax)?

> And in every case (_including_ that last case), %rax has a very
> interesting pattern.. That's the (bad) buf->ops pointer that  was
> loaded from the somehow corrupted "buf".

> So _if_ this is some kind of use-after-free thing, and the allocation
> got re-used for something else, that might just be related to whatever
> ends up being the offset that is filled in with the (int) error
> number.
> 
> Except the offset is that %r12*0x28+0x10, so we're talking a byte
> offset of 330 bytes into the allocation, and apparently the eight
> previous (0-7) iterations were fine.
> 
> Which is really odd.

I wonder what pipe->buffers is equal to here...

> I'm not seeing anything that makes sense. I'll have to think about this.
> 
> I'm assuming you don't have slub debugging enabled, and no way to
> enable it and try to catch this?

FWIW, I would try to slap
if (buf->ops && (unsigned long)buf->ops <= 0x)
dump the living hell out of that thing
and see what it catches...

Re: Kernel crash in free_pipe_info()

[Al Viro]

On Mon, Oct 30, 2017 at 07:08:46PM -0700, Linus Torvalds wrote:
> On Mon, Oct 30, 2017 at 6:19 PM, Cong Wang  wrote:
> >
> > 1. The faulty addresses are all near 0001, with one exception
> > of null (which is the most recent one)
> 
> Well, they're at 8(%rax), except for that last case.

0x10(%rax)?

> And in every case (_including_ that last case), %rax has a very
> interesting pattern.. That's the (bad) buf->ops pointer that  was
> loaded from the somehow corrupted "buf".

> So _if_ this is some kind of use-after-free thing, and the allocation
> got re-used for something else, that might just be related to whatever
> ends up being the offset that is filled in with the (int) error
> number.
> 
> Except the offset is that %r12*0x28+0x10, so we're talking a byte
> offset of 330 bytes into the allocation, and apparently the eight
> previous (0-7) iterations were fine.
> 
> Which is really odd.

I wonder what pipe->buffers is equal to here...

> I'm not seeing anything that makes sense. I'll have to think about this.
> 
> I'm assuming you don't have slub debugging enabled, and no way to
> enable it and try to catch this?

FWIW, I would try to slap
if (buf->ops && (unsigned long)buf->ops <= 0x)
dump the living hell out of that thing
and see what it catches...

Re: [PATCH v6 00/10] rockchip: kevin: Enable edp display

[JeffyChen]

Hi Heiko,

On 10/31/2017 07:01 AM, Heiko Stuebner wrote:

As I was just looking at the edp dts change in patch1 again, does this
series also contain a fix for the issue below [0] ?

I'm still seeing this on 4.14-rc6 with the most recent drm tree merged in.

i saw that too, it should due to our psr code...i think Zain has solved 
these in chromeos kernel, i will ask Zain if he have time to upstream 
them, or maybe i'll try to upstream them.


Heiko

[0]

[   27.960120] BUG: scheduling while atomic: kworker/1:1/68/0x0002
[   27.974429] Modules linked in: rockchipdrm dw_hdmi analogix_dp 
drm_kms_helper panel_simple crc32_ce drm crct10dif_ce rockchip_saradc pwm_bl 
pwm_cros_ec rockchip_thermal ip_tables x_tabl
es ipv6 smsc95xx smsc75xx ax88179_178a asix usbnet phy_rockchip_pcie 
pcie_rockchip
[   28.008769] CPU: 1 PID: 68 Comm: kworker/1:1 Tainted: GW   
4.14.0-rc7-03201-g12490811b353 #559
[   28.008774] Hardware name: Google Kevin (DT)
[   28.008825] Workqueue: events analogix_dp_psr_work [rockchipdrm]
[   28.008828] Call trace:
[   28.008838] [] dump_backtrace+0x0/0x378
[   28.008842] [] show_stack+0x14/0x20
[   28.008847] [] dump_stack+0x9c/0xbc
[   28.008852] [] __schedule_bug+0x4c/0x70
[   28.008856] [] __schedule+0x558/0x5e8
[   28.008859] [] schedule+0x38/0xa0
[   28.008864] [] schedule_hrtimeout_range_clock+0x84/0xe8
[   28.008867] [] schedule_hrtimeout_range+0x10/0x18
[   28.008870] [] usleep_range+0x64/0x78
[   28.008882] [] analogix_dp_transfer+0x16c/0xa88 
[analogix_dp]
[   28.008891] [] analogix_dpaux_transfer+0x10/0x18 
[analogix_dp]
[   28.008950] [] drm_dp_dpcd_access+0x4c/0xf8 
[drm_kms_helper]
[   28.008994] [] drm_dp_dpcd_write+0x1c/0x28 [drm_kms_helper]
[   28.009002] [] analogix_dp_disable_psr+0x60/0xb0 
[analogix_dp]
[   28.009036] [] analogix_dp_psr_work+0x4c/0xc0 [rockchipdrm]
[   28.009040] [] process_one_work+0x1d4/0x348
[   28.009043] [] worker_thread+0x48/0x470
[   28.009048] [] kthread+0x12c/0x130
[   28.009052] [] ret_from_fork+0x10/0x18

Re: [PATCH v6 00/10] rockchip: kevin: Enable edp display

[JeffyChen]

Hi Heiko,

On 10/31/2017 07:01 AM, Heiko Stuebner wrote:

As I was just looking at the edp dts change in patch1 again, does this
series also contain a fix for the issue below [0] ?

I'm still seeing this on 4.14-rc6 with the most recent drm tree merged in.

i saw that too, it should due to our psr code...i think Zain has solved 
these in chromeos kernel, i will ask Zain if he have time to upstream 
them, or maybe i'll try to upstream them.


Heiko

[0]

[   27.960120] BUG: scheduling while atomic: kworker/1:1/68/0x0002
[   27.974429] Modules linked in: rockchipdrm dw_hdmi analogix_dp 
drm_kms_helper panel_simple crc32_ce drm crct10dif_ce rockchip_saradc pwm_bl 
pwm_cros_ec rockchip_thermal ip_tables x_tabl
es ipv6 smsc95xx smsc75xx ax88179_178a asix usbnet phy_rockchip_pcie 
pcie_rockchip
[   28.008769] CPU: 1 PID: 68 Comm: kworker/1:1 Tainted: GW   
4.14.0-rc7-03201-g12490811b353 #559
[   28.008774] Hardware name: Google Kevin (DT)
[   28.008825] Workqueue: events analogix_dp_psr_work [rockchipdrm]
[   28.008828] Call trace:
[   28.008838] [] dump_backtrace+0x0/0x378
[   28.008842] [] show_stack+0x14/0x20
[   28.008847] [] dump_stack+0x9c/0xbc
[   28.008852] [] __schedule_bug+0x4c/0x70
[   28.008856] [] __schedule+0x558/0x5e8
[   28.008859] [] schedule+0x38/0xa0
[   28.008864] [] schedule_hrtimeout_range_clock+0x84/0xe8
[   28.008867] [] schedule_hrtimeout_range+0x10/0x18
[   28.008870] [] usleep_range+0x64/0x78
[   28.008882] [] analogix_dp_transfer+0x16c/0xa88 
[analogix_dp]
[   28.008891] [] analogix_dpaux_transfer+0x10/0x18 
[analogix_dp]
[   28.008950] [] drm_dp_dpcd_access+0x4c/0xf8 
[drm_kms_helper]
[   28.008994] [] drm_dp_dpcd_write+0x1c/0x28 [drm_kms_helper]
[   28.009002] [] analogix_dp_disable_psr+0x60/0xb0 
[analogix_dp]
[   28.009036] [] analogix_dp_psr_work+0x4c/0xc0 [rockchipdrm]
[   28.009040] [] process_one_work+0x1d4/0x348
[   28.009043] [] worker_thread+0x48/0x470
[   28.009048] [] kthread+0x12c/0x130
[   28.009052] [] ret_from_fork+0x10/0x18

Re: [PATCH V1] rpmsg: glink: Initialize the "intent_req_comp" completion variable

[Bjorn Andersson]

On Sun 29 Oct 22:41 PDT 2017, Arun Kumar Neelakantam wrote:

> The "intent_req_comp" variable is used without initialization which
> results in NULL pointer dereference in qcom_glink_request_intent().
> 
> we need to initialize the completion variable before using it.
> 
> Fixes: 27b9c5b66b23 ("rpmsg: glink: Request for intents when unavailable")
> Signed-off-by: Arun Kumar Neelakantam 

Thanks, applied.

Regards,
Bjorn

> ---
>  drivers/rpmsg/qcom_glink_native.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c 
> b/drivers/rpmsg/qcom_glink_native.c
> index 5dcc9bf..fcd46ab 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -227,6 +227,7 @@ static struct glink_channel 
> *qcom_glink_alloc_channel(struct qcom_glink *glink,
>  
>   init_completion(>open_req);
>   init_completion(>open_ack);
> + init_completion(>intent_req_comp);
>  
>   INIT_LIST_HEAD(>done_intents);
>   INIT_WORK(>intent_work, qcom_glink_rx_done_work);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH V1] rpmsg: glink: Initialize the "intent_req_comp" completion variable

[Bjorn Andersson]

On Sun 29 Oct 22:41 PDT 2017, Arun Kumar Neelakantam wrote:

> The "intent_req_comp" variable is used without initialization which
> results in NULL pointer dereference in qcom_glink_request_intent().
> 
> we need to initialize the completion variable before using it.
> 
> Fixes: 27b9c5b66b23 ("rpmsg: glink: Request for intents when unavailable")
> Signed-off-by: Arun Kumar Neelakantam 

Thanks, applied.

Regards,
Bjorn

> ---
>  drivers/rpmsg/qcom_glink_native.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/rpmsg/qcom_glink_native.c 
> b/drivers/rpmsg/qcom_glink_native.c
> index 5dcc9bf..fcd46ab 100644
> --- a/drivers/rpmsg/qcom_glink_native.c
> +++ b/drivers/rpmsg/qcom_glink_native.c
> @@ -227,6 +227,7 @@ static struct glink_channel 
> *qcom_glink_alloc_channel(struct qcom_glink *glink,
>  
>   init_completion(>open_req);
>   init_completion(>open_ack);
> + init_completion(>intent_req_comp);
>  
>   INIT_LIST_HEAD(>done_intents);
>   INIT_WORK(>intent_work, qcom_glink_rx_done_work);
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH v2 0/3] GLINK intent preallocation support

[Bjorn Andersson]

On Thu 26 Oct 15:28 PDT 2017, Chris Lew wrote:

> Intents are used to specify when a channel can receive data from a
> remoteproc. Add support for channels to customize the size and amount
> of prequeued intents.
> 
> An audio channel might expect to receive 3 packets of size 4k in rapid
> succession. This change allows the channel to prepare for this data instead
> of allocating on demand.
> 

Thanks, tested and applied with some minor modifications to patch 2.

Regards,
Bjorn

> Chris Lew (3):
>   dt-bindings: soc: qcom: Support GLINK intents
>   rpmsg: glink: Add support to preallocate intents
>   rpmsg: glink: Use best fit intent during tx
> 
>  .../devicetree/bindings/soc/qcom/qcom,glink.txt| 10 +
>  drivers/rpmsg/qcom_glink_native.c  | 48 
> +-
>  2 files changed, 47 insertions(+), 11 deletions(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH v2 0/3] GLINK intent preallocation support

[Bjorn Andersson]

On Thu 26 Oct 15:28 PDT 2017, Chris Lew wrote:

> Intents are used to specify when a channel can receive data from a
> remoteproc. Add support for channels to customize the size and amount
> of prequeued intents.
> 
> An audio channel might expect to receive 3 packets of size 4k in rapid
> succession. This change allows the channel to prepare for this data instead
> of allocating on demand.
> 

Thanks, tested and applied with some minor modifications to patch 2.

Regards,
Bjorn

> Chris Lew (3):
>   dt-bindings: soc: qcom: Support GLINK intents
>   rpmsg: glink: Add support to preallocate intents
>   rpmsg: glink: Use best fit intent during tx
> 
>  .../devicetree/bindings/soc/qcom/qcom,glink.txt| 10 +
>  drivers/rpmsg/qcom_glink_native.c  | 48 
> +-
>  2 files changed, 47 insertions(+), 11 deletions(-)
> 
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project
> 

Re: [PATCH] nilfs2: remove inode->i_version initialization

[Ryusuke Konishi]

On Mon, 30 Oct 2017 11:17:02 -0400, Jeff Layton wrote:
> From: Jeff Layton 
> 
> It's never used in nilfs2.
> 
> Signed-off-by: Jeff Layton 
> ---

Applied, thank you.

Ryusuke Konishi

>  fs/nilfs2/super.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 4fc018dfcfae..3ce20cd44a20 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -160,7 +160,6 @@ struct inode *nilfs_alloc_inode(struct super_block *sb)
>   ii->i_bh = NULL;
>   ii->i_state = 0;
>   ii->i_cno = 0;
> - ii->vfs_inode.i_version = 1;
>   nilfs_mapping_init(>i_btnode_cache, >vfs_inode);
>   return >vfs_inode;
>  }
> -- 
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] nilfs2: remove inode->i_version initialization

[Ryusuke Konishi]

On Mon, 30 Oct 2017 11:17:02 -0400, Jeff Layton wrote:
> From: Jeff Layton 
> 
> It's never used in nilfs2.
> 
> Signed-off-by: Jeff Layton 
> ---

Applied, thank you.

Ryusuke Konishi

>  fs/nilfs2/super.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 4fc018dfcfae..3ce20cd44a20 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -160,7 +160,6 @@ struct inode *nilfs_alloc_inode(struct super_block *sb)
>   ii->i_bh = NULL;
>   ii->i_state = 0;
>   ii->i_cno = 0;
> - ii->vfs_inode.i_version = 1;
>   nilfs_mapping_init(>i_btnode_cache, >vfs_inode);
>   return >vfs_inode;
>  }
> -- 
> 2.13.6
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/2] remoteproc: increase debug capabilities

[Bjorn Andersson]

On Fri 27 Oct 05:41 PDT 2017, Loic Pallardy wrote:

> This series increases remoteproc debug capabilities by adding:
> - associated resource table dump feature
> - registered carveouts list dump feature
> 

This looks very reasonable, can you please fix the problem reported by
0-day?

Regards,
Bjorn

> Loic Pallardy (2):
>   remoteproc: debug: add resource table dump feature
>   remoteproc: debug: add carveouts list dump feature
> 
>  drivers/remoteproc/remoteproc_debugfs.c | 130 
> 
>  1 file changed, 130 insertions(+)
> 
> -- 
> 1.9.1
> 

Re: [PATCH 0/2] remoteproc: increase debug capabilities

[Bjorn Andersson]

On Fri 27 Oct 05:41 PDT 2017, Loic Pallardy wrote:

> This series increases remoteproc debug capabilities by adding:
> - associated resource table dump feature
> - registered carveouts list dump feature
> 

This looks very reasonable, can you please fix the problem reported by
0-day?

Regards,
Bjorn

> Loic Pallardy (2):
>   remoteproc: debug: add resource table dump feature
>   remoteproc: debug: add carveouts list dump feature
> 
>  drivers/remoteproc/remoteproc_debugfs.c | 130 
> 
>  1 file changed, 130 insertions(+)
> 
> -- 
> 1.9.1
> 

Re: [PATCH] platform/x86: fujitsu-laptop: Fix radio LED detection

[Michał Kępień]

> > Do you consider this an important fix? We are at -rc7 now, I'm not
> > sure it's so critical. Tell me if you consider otherwise.
> 
> I agree - from my perspective I wouldn't have thought it so critical as to
> push it out this late in the development cycle.  It's not a regression as
> such and is largely cosmetic.  Others may argue differently though.

I agree as well.  The erroneous log messages will only be generated when
any rfkill device in the system is enabled or disabled, so IMHO this is
mostly a nuisance thus can be handled when convenient.

-- 
Best regards,
Michał Kępień

Re: [PATCH] platform/x86: fujitsu-laptop: Fix radio LED detection

[Michał Kępień]

> > Do you consider this an important fix? We are at -rc7 now, I'm not
> > sure it's so critical. Tell me if you consider otherwise.
> 
> I agree - from my perspective I wouldn't have thought it so critical as to
> push it out this late in the development cycle.  It's not a regression as
> such and is largely cosmetic.  Others may argue differently though.

I agree as well.  The erroneous log messages will only be generated when
any rfkill device in the system is enabled or disabled, so IMHO this is
mostly a nuisance thus can be handled when convenient.

-- 
Best regards,
Michał Kępień

Re: [PATCH] net: caif: chnl_net: remove unnecessary null check in chnl_recv_cb

[Gustavo A. R. Silva]

Quoting "Gustavo A. R. Silva" :


Quoting "Gustavo A. R. Silva" :


Hi,

Quoting "Gustavo A. R. Silva" :


container_of is never null, so this null check is unnecessary.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
net/caif/chnl_net.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 922ac1d..489298d 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -77,8 +77,6 @@ static int chnl_recv_cb(struct cflayer *layr,  
struct cfpkt *pkt)

u8 buf;

priv = container_of(layr, struct chnl_net, chnl);
-   if (!priv)
-   return -EINVAL;


But the driver only passes in to container_of the type of driver  
structure registered with it. So this type of manipulation must be  
completely safe. Right?


Now it seems to me (again) that the patch is fine.

I'm sorry, I'm a little bit confused here.



skb = (struct sk_buff *) cfpkt_tonative(pkt);

--
2.7.4


Please, ignore this patch.

I just realized that function chnl_recv_cb is being called only  
during initialization:


chnl_init_module() -> rtnl_link_register() -> ipcaif_net_setup() ->  
chnl_recv_cb():




Well, here ipcaif_net_setup stores a pointer to chnl_recv_cb in a  
structure. It doesn't call it.



static void ipcaif_net_setup(struct net_device *dev)
{
[...]
priv = netdev_priv(dev);
priv->chnl.receive = chnl_recv_cb;

[...]
}

static struct rtnl_link_ops ipcaif_link_ops __read_mostly = {
.kind   = "caif",
.priv_size  = sizeof(struct chnl_net),
.setup  = ipcaif_net_setup,
.maxtype= IFLA_CAIF_MAX,
.policy = ipcaif_policy,
.newlink= ipcaif_newlink,
.changelink = ipcaif_changelink,
.get_size   = ipcaif_get_size,
.fill_info  = ipcaif_fill_info,

};

static int __init chnl_init_module(void)
{
return rtnl_link_register(_link_ops);
}





--
Gustavo A. R. Silva

Re: [PATCH] net: caif: chnl_net: remove unnecessary null check in chnl_recv_cb

[Gustavo A. R. Silva]

Quoting "Gustavo A. R. Silva" :


Quoting "Gustavo A. R. Silva" :


Hi,

Quoting "Gustavo A. R. Silva" :


container_of is never null, so this null check is unnecessary.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
net/caif/chnl_net.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 922ac1d..489298d 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -77,8 +77,6 @@ static int chnl_recv_cb(struct cflayer *layr,  
struct cfpkt *pkt)

u8 buf;

priv = container_of(layr, struct chnl_net, chnl);
-   if (!priv)
-   return -EINVAL;


But the driver only passes in to container_of the type of driver  
structure registered with it. So this type of manipulation must be  
completely safe. Right?


Now it seems to me (again) that the patch is fine.

I'm sorry, I'm a little bit confused here.



skb = (struct sk_buff *) cfpkt_tonative(pkt);

--
2.7.4


Please, ignore this patch.

I just realized that function chnl_recv_cb is being called only  
during initialization:


chnl_init_module() -> rtnl_link_register() -> ipcaif_net_setup() ->  
chnl_recv_cb():




Well, here ipcaif_net_setup stores a pointer to chnl_recv_cb in a  
structure. It doesn't call it.



static void ipcaif_net_setup(struct net_device *dev)
{
[...]
priv = netdev_priv(dev);
priv->chnl.receive = chnl_recv_cb;

[...]
}

static struct rtnl_link_ops ipcaif_link_ops __read_mostly = {
.kind   = "caif",
.priv_size  = sizeof(struct chnl_net),
.setup  = ipcaif_net_setup,
.maxtype= IFLA_CAIF_MAX,
.policy = ipcaif_policy,
.newlink= ipcaif_newlink,
.changelink = ipcaif_changelink,
.get_size   = ipcaif_get_size,
.fill_info  = ipcaif_fill_info,

};

static int __init chnl_init_module(void)
{
return rtnl_link_register(_link_ops);
}





--
Gustavo A. R. Silva

linux-next: no tree for October 31

[Stephen Rothwell]

Hi all,

There will be no linux-next tree for today (October 31) as jetlag, a
cold and food poisoning is a bad combination :-(

Hopefully I will restart linux-next tomorrow (November 1).

-- 
Cheers,
Stephen Rothwell

linux-next: no tree for October 31

[Stephen Rothwell]

Hi all,

There will be no linux-next tree for today (October 31) as jetlag, a
cold and food poisoning is a bad combination :-(

Hopefully I will restart linux-next tomorrow (November 1).

-- 
Cheers,
Stephen Rothwell

Re: linux-next: No tree for Oct 20th

[Stephen Rothwell]

Hi Mark,

On Mon, 30 Oct 2017 21:32:02 + Mark Brown  wrote:
>
> There will be no -next tree today, there are too many non-trivial
> conflicts for things to complete in a reasonable time.

Thanks very much for you efforts while I was traveling. I am back now
and will restart linux-next tomorrow (Nov 1).

-- 
Cheers,
Stephen Rothwell

Re: linux-next: No tree for Oct 20th

[Stephen Rothwell]

Hi Mark,

On Mon, 30 Oct 2017 21:32:02 + Mark Brown  wrote:
>
> There will be no -next tree today, there are too many non-trivial
> conflicts for things to complete in a reasonable time.

Thanks very much for you efforts while I was traveling. I am back now
and will restart linux-next tomorrow (Nov 1).

-- 
Cheers,
Stephen Rothwell

[PATCH 2/2] f2fs: support quota sys files

[Jaegeuk Kim]

This patch supports hidden quota files in the system, which will be used for
Android. It requires up-to-date f2fs-tools later than v1.9.0.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/checkpoint.c |   9 +++-
 fs/f2fs/f2fs.h   |   9 +++-
 fs/f2fs/recovery.c   |   8 ++-
 fs/f2fs/super.c  | 145 ++-
 4 files changed, 153 insertions(+), 18 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 6b52d4b66c7b..78e1b2998bbd 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -615,6 +615,9 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
block_t start_blk, orphan_blocks, i, j;
unsigned int s_flags = sbi->sb->s_flags;
int err = 0;
+#ifdef CONFIG_QUOTA
+   int quota_enabled;
+#endif
 
if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
return 0;
@@ -627,8 +630,9 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 #ifdef CONFIG_QUOTA
/* Needed for iput() to work correctly and not trash data */
sbi->sb->s_flags |= MS_ACTIVE;
+
/* Turn on quotas so that they are updated correctly */
-   f2fs_enable_quota_files(sbi);
+   quota_enabled = f2fs_enable_quota_files(sbi, s_flags & MS_RDONLY);
 #endif
 
start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
@@ -656,7 +660,8 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 out:
 #ifdef CONFIG_QUOTA
/* Turn quotas off */
-   f2fs_quota_off_umount(sbi->sb);
+   if (quota_enabled)
+   f2fs_quota_off_umount(sbi->sb);
 #endif
sbi->sb->s_flags = s_flags; /* Restore MS_RDONLY status */
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9a1c7ffa6845..e1d3a940d9f8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1384,6 +1384,13 @@ static inline unsigned long long cur_cp_version(struct 
f2fs_checkpoint *cp)
return le64_to_cpu(cp->checkpoint_ver);
 }
 
+static inline unsigned long f2fs_qf_ino(struct super_block *sb, int type)
+{
+   if (type < F2FS_QUOTA_INO)
+   return le32_to_cpu(F2FS_SB(sb)->raw_super->qf_ino[type]);
+   return 0;
+}
+
 static inline __u64 cur_cp_crc(struct f2fs_checkpoint *cp)
 {
size_t crc_offset = le32_to_cpu(cp->checksum_offset);
@@ -2526,7 +2533,7 @@ static inline int f2fs_add_link(struct dentry *dentry, 
struct inode *inode)
  */
 int f2fs_inode_dirtied(struct inode *inode, bool sync);
 void f2fs_inode_synced(struct inode *inode);
-void f2fs_enable_quota_files(struct f2fs_sb_info *sbi);
+int f2fs_enable_quota_files(struct f2fs_sb_info *sbi, bool rdonly);
 void f2fs_quota_off_umount(struct super_block *sb);
 int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover);
 int f2fs_sync_fs(struct super_block *sb, int sync);
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 9626758bc762..92c57ace1939 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -594,6 +594,9 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool 
check_only)
int ret = 0;
unsigned long s_flags = sbi->sb->s_flags;
bool need_writecp = false;
+#ifdef CONFIG_QUOTA
+   int quota_enabled;
+#endif
 
if (s_flags & MS_RDONLY) {
f2fs_msg(sbi->sb, KERN_INFO, "orphan cleanup on readonly fs");
@@ -604,7 +607,7 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool 
check_only)
/* Needed for iput() to work correctly and not trash data */
sbi->sb->s_flags |= MS_ACTIVE;
/* Turn on quotas so that they are updated correctly */
-   f2fs_enable_quota_files(sbi);
+   quota_enabled = f2fs_enable_quota_files(sbi, s_flags & MS_RDONLY);
 #endif
 
fsync_entry_slab = f2fs_kmem_cache_create("f2fs_fsync_inode_entry",
@@ -665,7 +668,8 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool 
check_only)
 out:
 #ifdef CONFIG_QUOTA
/* Turn quotas off */
-   f2fs_quota_off_umount(sbi->sb);
+   if (quota_enabled)
+   f2fs_quota_off_umount(sbi->sb);
 #endif
sbi->sb->s_flags = s_flags; /* Restore MS_RDONLY status */
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 96e145c34ba2..0ca7b055e4e0 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -213,6 +213,12 @@ static int f2fs_set_qf_name(struct super_block *sb, int 
qtype,
"quota options when quota turned on");
return -EINVAL;
}
+   if (f2fs_sb_has_quota_ino(sb)) {
+   f2fs_msg(sb, KERN_INFO,
+   "QUOTA feature is enabled, so ignore qf_name");
+   return 0;
+   }
+
qname = match_strdup(args);
if (!qname) {
f2fs_msg(sb, KERN_ERR,
@@ -291,6 +297,18 @@ static int f2fs_check_quota_options(struct f2fs_sb_info 
*sbi)
return -1;
}
}
+
+   if (f2fs_sb_has_quota_ino(sbi->sb) && sbi->s_jquota_fmt) {
+   f2fs_msg(sbi->sb, KERN_INFO,
+ 

[PATCH 2/2] f2fs: support quota sys files

[Jaegeuk Kim]

This patch supports hidden quota files in the system, which will be used for
Android. It requires up-to-date f2fs-tools later than v1.9.0.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/checkpoint.c |   9 +++-
 fs/f2fs/f2fs.h   |   9 +++-
 fs/f2fs/recovery.c   |   8 ++-
 fs/f2fs/super.c  | 145 ++-
 4 files changed, 153 insertions(+), 18 deletions(-)

diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
index 6b52d4b66c7b..78e1b2998bbd 100644
--- a/fs/f2fs/checkpoint.c
+++ b/fs/f2fs/checkpoint.c
@@ -615,6 +615,9 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
block_t start_blk, orphan_blocks, i, j;
unsigned int s_flags = sbi->sb->s_flags;
int err = 0;
+#ifdef CONFIG_QUOTA
+   int quota_enabled;
+#endif
 
if (!is_set_ckpt_flags(sbi, CP_ORPHAN_PRESENT_FLAG))
return 0;
@@ -627,8 +630,9 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 #ifdef CONFIG_QUOTA
/* Needed for iput() to work correctly and not trash data */
sbi->sb->s_flags |= MS_ACTIVE;
+
/* Turn on quotas so that they are updated correctly */
-   f2fs_enable_quota_files(sbi);
+   quota_enabled = f2fs_enable_quota_files(sbi, s_flags & MS_RDONLY);
 #endif
 
start_blk = __start_cp_addr(sbi) + 1 + __cp_payload(sbi);
@@ -656,7 +660,8 @@ int recover_orphan_inodes(struct f2fs_sb_info *sbi)
 out:
 #ifdef CONFIG_QUOTA
/* Turn quotas off */
-   f2fs_quota_off_umount(sbi->sb);
+   if (quota_enabled)
+   f2fs_quota_off_umount(sbi->sb);
 #endif
sbi->sb->s_flags = s_flags; /* Restore MS_RDONLY status */
 
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9a1c7ffa6845..e1d3a940d9f8 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -1384,6 +1384,13 @@ static inline unsigned long long cur_cp_version(struct 
f2fs_checkpoint *cp)
return le64_to_cpu(cp->checkpoint_ver);
 }
 
+static inline unsigned long f2fs_qf_ino(struct super_block *sb, int type)
+{
+   if (type < F2FS_QUOTA_INO)
+   return le32_to_cpu(F2FS_SB(sb)->raw_super->qf_ino[type]);
+   return 0;
+}
+
 static inline __u64 cur_cp_crc(struct f2fs_checkpoint *cp)
 {
size_t crc_offset = le32_to_cpu(cp->checksum_offset);
@@ -2526,7 +2533,7 @@ static inline int f2fs_add_link(struct dentry *dentry, 
struct inode *inode)
  */
 int f2fs_inode_dirtied(struct inode *inode, bool sync);
 void f2fs_inode_synced(struct inode *inode);
-void f2fs_enable_quota_files(struct f2fs_sb_info *sbi);
+int f2fs_enable_quota_files(struct f2fs_sb_info *sbi, bool rdonly);
 void f2fs_quota_off_umount(struct super_block *sb);
 int f2fs_commit_super(struct f2fs_sb_info *sbi, bool recover);
 int f2fs_sync_fs(struct super_block *sb, int sync);
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index 9626758bc762..92c57ace1939 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -594,6 +594,9 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool 
check_only)
int ret = 0;
unsigned long s_flags = sbi->sb->s_flags;
bool need_writecp = false;
+#ifdef CONFIG_QUOTA
+   int quota_enabled;
+#endif
 
if (s_flags & MS_RDONLY) {
f2fs_msg(sbi->sb, KERN_INFO, "orphan cleanup on readonly fs");
@@ -604,7 +607,7 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool 
check_only)
/* Needed for iput() to work correctly and not trash data */
sbi->sb->s_flags |= MS_ACTIVE;
/* Turn on quotas so that they are updated correctly */
-   f2fs_enable_quota_files(sbi);
+   quota_enabled = f2fs_enable_quota_files(sbi, s_flags & MS_RDONLY);
 #endif
 
fsync_entry_slab = f2fs_kmem_cache_create("f2fs_fsync_inode_entry",
@@ -665,7 +668,8 @@ int recover_fsync_data(struct f2fs_sb_info *sbi, bool 
check_only)
 out:
 #ifdef CONFIG_QUOTA
/* Turn quotas off */
-   f2fs_quota_off_umount(sbi->sb);
+   if (quota_enabled)
+   f2fs_quota_off_umount(sbi->sb);
 #endif
sbi->sb->s_flags = s_flags; /* Restore MS_RDONLY status */
 
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 96e145c34ba2..0ca7b055e4e0 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -213,6 +213,12 @@ static int f2fs_set_qf_name(struct super_block *sb, int 
qtype,
"quota options when quota turned on");
return -EINVAL;
}
+   if (f2fs_sb_has_quota_ino(sb)) {
+   f2fs_msg(sb, KERN_INFO,
+   "QUOTA feature is enabled, so ignore qf_name");
+   return 0;
+   }
+
qname = match_strdup(args);
if (!qname) {
f2fs_msg(sb, KERN_ERR,
@@ -291,6 +297,18 @@ static int f2fs_check_quota_options(struct f2fs_sb_info 
*sbi)
return -1;
}
}
+
+   if (f2fs_sb_has_quota_ino(sbi->sb) && sbi->s_jquota_fmt) {
+   f2fs_msg(sbi->sb, KERN_INFO,
+   "QUOTA feature is 

[PATCH 1/2] f2fs: add quota_ino feature infra

[Jaegeuk Kim]

This patch adds quota_ino feature infra to be used for quota files.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/f2fs.h  | 6 ++
 fs/f2fs/sysfs.c | 7 +++
 include/linux/f2fs_fs.h | 6 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4a75f07f1dc8..9a1c7ffa6845 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -122,6 +122,7 @@ struct f2fs_mount_info {
 #define F2FS_FEATURE_PRJQUOTA  0x0010
 #define F2FS_FEATURE_INODE_CHKSUM  0x0020
 #define F2FS_FEATURE_FLEXIBLE_INLINE_XATTR 0x0040
+#define F2FS_FEATURE_QUOTA_INO 0x0080
 
 #define F2FS_HAS_FEATURE(sb, mask) \
((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -3070,6 +3071,11 @@ static inline int 
f2fs_sb_has_flexible_inline_xattr(struct super_block *sb)
return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_FLEXIBLE_INLINE_XATTR);
 }
 
+static inline int f2fs_sb_has_quota_ino(struct super_block *sb)
+{
+   return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_QUOTA_INO);
+}
+
 #ifdef CONFIG_BLK_DEV_ZONED
 static inline int get_blkz_type(struct f2fs_sb_info *sbi,
struct block_device *bdev, block_t blkaddr)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index f0fdc89ce82f..9835348b6e5d 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -110,6 +110,9 @@ static ssize_t features_show(struct f2fs_attr *a,
if (f2fs_sb_has_flexible_inline_xattr(sb))
len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "flexible_inline_xattr");
+   if (f2fs_sb_has_quota_ino(sb))
+   len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
+   len ? ", " : "", "quota_ino");
len += snprintf(buf + len, PAGE_SIZE - len, "\n");
return len;
 }
@@ -227,6 +230,7 @@ enum feat_id {
FEAT_PROJECT_QUOTA,
FEAT_INODE_CHECKSUM,
FEAT_FLEXIBLE_INLINE_XATTR,
+   FEAT_QUOTA_INO,
 };
 
 static ssize_t f2fs_feature_show(struct f2fs_attr *a,
@@ -240,6 +244,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
case FEAT_PROJECT_QUOTA:
case FEAT_INODE_CHECKSUM:
case FEAT_FLEXIBLE_INLINE_XATTR:
+   case FEAT_QUOTA_INO:
return snprintf(buf, PAGE_SIZE, "supported\n");
}
return 0;
@@ -314,6 +319,7 @@ F2FS_FEATURE_RO_ATTR(extra_attr, FEAT_EXTRA_ATTR);
 F2FS_FEATURE_RO_ATTR(project_quota, FEAT_PROJECT_QUOTA);
 F2FS_FEATURE_RO_ATTR(inode_checksum, FEAT_INODE_CHECKSUM);
 F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, FEAT_FLEXIBLE_INLINE_XATTR);
+F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
 
 #define ATTR_LIST(name) (_attr_##name.attr)
 static struct attribute *f2fs_attrs[] = {
@@ -364,6 +370,7 @@ static struct attribute *f2fs_feat_attrs[] = {
ATTR_LIST(project_quota),
ATTR_LIST(inode_checksum),
ATTR_LIST(flexible_inline_xattr),
+   ATTR_LIST(quota_ino),
NULL,
 };
 
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 50a8ee501bf1..ce34007972c3 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -36,6 +36,9 @@
 #define F2FS_NODE_INO(sbi) ((sbi)->node_ino_num)
 #define F2FS_META_INO(sbi) ((sbi)->meta_ino_num)
 
+#define F2FS_QUOTA_INO 3
+#define F2FS_MAX_QUOTAS3
+
 #define F2FS_IO_SIZE(sbi)  (1 << (sbi)->write_io_size_bits) /* Blocks */
 #define F2FS_IO_SIZE_KB(sbi)   (1 << ((sbi)->write_io_size_bits + 2)) /* KB */
 #define F2FS_IO_SIZE_BYTES(sbi)(1 << ((sbi)->write_io_size_bits + 12)) 
/* B */
@@ -108,7 +111,8 @@ struct f2fs_super_block {
__u8 encryption_level;  /* versioning level for encryption */
__u8 encrypt_pw_salt[16];   /* Salt used for string2key algorithm */
struct f2fs_device devs[MAX_DEVICES];   /* device list */
-   __u8 reserved[327]; /* valid reserved region */
+   __le32 qf_ino[F2FS_MAX_QUOTAS]; /* quota inode numbers */
+   __u8 reserved[315]; /* valid reserved region */
 } __packed;
 
 /*
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

[PATCH 1/2] f2fs: add quota_ino feature infra

[Jaegeuk Kim]

This patch adds quota_ino feature infra to be used for quota files.

Signed-off-by: Jaegeuk Kim 
---
 fs/f2fs/f2fs.h  | 6 ++
 fs/f2fs/sysfs.c | 7 +++
 include/linux/f2fs_fs.h | 6 +-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 4a75f07f1dc8..9a1c7ffa6845 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -122,6 +122,7 @@ struct f2fs_mount_info {
 #define F2FS_FEATURE_PRJQUOTA  0x0010
 #define F2FS_FEATURE_INODE_CHKSUM  0x0020
 #define F2FS_FEATURE_FLEXIBLE_INLINE_XATTR 0x0040
+#define F2FS_FEATURE_QUOTA_INO 0x0080
 
 #define F2FS_HAS_FEATURE(sb, mask) \
((F2FS_SB(sb)->raw_super->feature & cpu_to_le32(mask)) != 0)
@@ -3070,6 +3071,11 @@ static inline int 
f2fs_sb_has_flexible_inline_xattr(struct super_block *sb)
return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_FLEXIBLE_INLINE_XATTR);
 }
 
+static inline int f2fs_sb_has_quota_ino(struct super_block *sb)
+{
+   return F2FS_HAS_FEATURE(sb, F2FS_FEATURE_QUOTA_INO);
+}
+
 #ifdef CONFIG_BLK_DEV_ZONED
 static inline int get_blkz_type(struct f2fs_sb_info *sbi,
struct block_device *bdev, block_t blkaddr)
diff --git a/fs/f2fs/sysfs.c b/fs/f2fs/sysfs.c
index f0fdc89ce82f..9835348b6e5d 100644
--- a/fs/f2fs/sysfs.c
+++ b/fs/f2fs/sysfs.c
@@ -110,6 +110,9 @@ static ssize_t features_show(struct f2fs_attr *a,
if (f2fs_sb_has_flexible_inline_xattr(sb))
len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
len ? ", " : "", "flexible_inline_xattr");
+   if (f2fs_sb_has_quota_ino(sb))
+   len += snprintf(buf + len, PAGE_SIZE - len, "%s%s",
+   len ? ", " : "", "quota_ino");
len += snprintf(buf + len, PAGE_SIZE - len, "\n");
return len;
 }
@@ -227,6 +230,7 @@ enum feat_id {
FEAT_PROJECT_QUOTA,
FEAT_INODE_CHECKSUM,
FEAT_FLEXIBLE_INLINE_XATTR,
+   FEAT_QUOTA_INO,
 };
 
 static ssize_t f2fs_feature_show(struct f2fs_attr *a,
@@ -240,6 +244,7 @@ static ssize_t f2fs_feature_show(struct f2fs_attr *a,
case FEAT_PROJECT_QUOTA:
case FEAT_INODE_CHECKSUM:
case FEAT_FLEXIBLE_INLINE_XATTR:
+   case FEAT_QUOTA_INO:
return snprintf(buf, PAGE_SIZE, "supported\n");
}
return 0;
@@ -314,6 +319,7 @@ F2FS_FEATURE_RO_ATTR(extra_attr, FEAT_EXTRA_ATTR);
 F2FS_FEATURE_RO_ATTR(project_quota, FEAT_PROJECT_QUOTA);
 F2FS_FEATURE_RO_ATTR(inode_checksum, FEAT_INODE_CHECKSUM);
 F2FS_FEATURE_RO_ATTR(flexible_inline_xattr, FEAT_FLEXIBLE_INLINE_XATTR);
+F2FS_FEATURE_RO_ATTR(quota_ino, FEAT_QUOTA_INO);
 
 #define ATTR_LIST(name) (_attr_##name.attr)
 static struct attribute *f2fs_attrs[] = {
@@ -364,6 +370,7 @@ static struct attribute *f2fs_feat_attrs[] = {
ATTR_LIST(project_quota),
ATTR_LIST(inode_checksum),
ATTR_LIST(flexible_inline_xattr),
+   ATTR_LIST(quota_ino),
NULL,
 };
 
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
index 50a8ee501bf1..ce34007972c3 100644
--- a/include/linux/f2fs_fs.h
+++ b/include/linux/f2fs_fs.h
@@ -36,6 +36,9 @@
 #define F2FS_NODE_INO(sbi) ((sbi)->node_ino_num)
 #define F2FS_META_INO(sbi) ((sbi)->meta_ino_num)
 
+#define F2FS_QUOTA_INO 3
+#define F2FS_MAX_QUOTAS3
+
 #define F2FS_IO_SIZE(sbi)  (1 << (sbi)->write_io_size_bits) /* Blocks */
 #define F2FS_IO_SIZE_KB(sbi)   (1 << ((sbi)->write_io_size_bits + 2)) /* KB */
 #define F2FS_IO_SIZE_BYTES(sbi)(1 << ((sbi)->write_io_size_bits + 12)) 
/* B */
@@ -108,7 +111,8 @@ struct f2fs_super_block {
__u8 encryption_level;  /* versioning level for encryption */
__u8 encrypt_pw_salt[16];   /* Salt used for string2key algorithm */
struct f2fs_device devs[MAX_DEVICES];   /* device list */
-   __u8 reserved[327]; /* valid reserved region */
+   __le32 qf_ino[F2FS_MAX_QUOTAS]; /* quota inode numbers */
+   __u8 reserved[315]; /* valid reserved region */
 } __packed;
 
 /*
-- 
2.14.0.rc1.383.gd1ce394fe2-goog

[PATCH 1/1] Change ping_group_range default to what Android's init script sets.

[Rob Landley]

From: Rob Landley 

See message from the Android "native tools and libraries team" lead
(I.E. the maintainer of bionic, adb, toolbox, etc) at
http://lists.landley.net/pipermail/toybox-landley.net/2017-July/009103.html

Signed-off-by: Rob Landley 
---

 net/ipv4/af_inet.c |8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index e31108e..5b39a96 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1712,12 +1712,8 @@ static __net_init int inet_init_net(struct net *net)
net->ipv4.ip_local_ports.range[1] =  60999;
 
seqlock_init(>ipv4.ping_group_range.lock);
-   /*
-* Sane defaults - nobody may create ping sockets.
-* Boot scripts should set this to distro-specific group.
-*/
-   net->ipv4.ping_group_range.range[0] = make_kgid(_user_ns, 1);
-   net->ipv4.ping_group_range.range[1] = make_kgid(_user_ns, 0);
+   net->ipv4.ping_group_range.range[0] = make_kgid(_user_ns, 0);
+   net->ipv4.ping_group_range.range[1] = make_kgid(_user_ns, 
2147483647);
 
/* Default values for sysctl-controlled parameters.
 * We set them here, in case sysctl is not compiled.

[PATCH 1/1] Change ping_group_range default to what Android's init script sets.

[Rob Landley]

From: Rob Landley 

See message from the Android "native tools and libraries team" lead
(I.E. the maintainer of bionic, adb, toolbox, etc) at
http://lists.landley.net/pipermail/toybox-landley.net/2017-July/009103.html

Signed-off-by: Rob Landley 
---

 net/ipv4/af_inet.c |8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index e31108e..5b39a96 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1712,12 +1712,8 @@ static __net_init int inet_init_net(struct net *net)
net->ipv4.ip_local_ports.range[1] =  60999;
 
seqlock_init(>ipv4.ping_group_range.lock);
-   /*
-* Sane defaults - nobody may create ping sockets.
-* Boot scripts should set this to distro-specific group.
-*/
-   net->ipv4.ping_group_range.range[0] = make_kgid(_user_ns, 1);
-   net->ipv4.ping_group_range.range[1] = make_kgid(_user_ns, 0);
+   net->ipv4.ping_group_range.range[0] = make_kgid(_user_ns, 0);
+   net->ipv4.ping_group_range.range[1] = make_kgid(_user_ns, 
2147483647);
 
/* Default values for sysctl-controlled parameters.
 * We set them here, in case sysctl is not compiled.

[PATCH v2] mtd: nand: Fix writing mtdoops to nand flash.

[motobud]

From: Brent Taylor 

When mtdoops calls mtd_panic_write, it eventually calls
panic_nand_write in nand_base.c.  In order to properly
wait for the nand chip to be ready in panic_nand_wait,
the chip must first be selected.

When using the atmel nand flash controller, a panic
would occur due to a NULL pointer exception.

Signed-off-by: Brent Taylor 
---
Changes in v2:
  - drop deselecting the chip
  - move select_chip and panic_nand_wait after panic_nand_get_device

 drivers/mtd/nand/nand_base.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 12edaae17d81..2093667e2bcb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2802,12 +2802,16 @@ static int panic_nand_write(struct mtd_info *mtd, 
loff_t to, size_t len,
struct mtd_oob_ops ops;
int ret;
 
-   /* Wait for the device to get ready */
-   panic_nand_wait(mtd, chip, 400);
+   int chipnr = (int)(to >> chip->chip_shift);
 
/* Grab the device */
panic_nand_get_device(chip, mtd, FL_WRITING);
 
+   chip->select_chip(mtd, chipnr);
+
+   /* Wait for the device to get ready */
+   panic_nand_wait(mtd, chip, 400);
+
memset(, 0, sizeof(ops));
ops.len = len;
ops.datbuf = (uint8_t *)buf;
-- 
2.14.2

Re: [f2fs-dev] [PATCH v2] f2fs: add bug_on when f2fs_gc even fails to get one victim

[Yunlong Song]

So it seems it is really useful to add this bug_on in gc.

On 2017/10/31 11:17, Chao Yu wrote:

On 2017/10/31 9:32, Yunlong Song wrote:

I think there may be bugs somewhere, since no victim is selected but it
really needs gc.
What is the size of the data image?

I have providered the testcase, could you check that?


I can hit this bugon with generic/015 of fstest easily, could have a look at
this?

Anyway, I have looked into this issue at last weekend, forgot to post related
info. It shows that if the image size is very small, e.g. 50MB, current
segment will encroach overprov and reserved segments, so free segment count will
always be small than reserved segment threshold, result in triggering bugon in
FGGC.

[SB: 1] [CP: 2] [SIT: 2] [NAT: 2] [SSA: 1] [MAIN: 17(OverProv:14 Resv:12)]

   - Valid: 6
   - Dirty: 0
   - Prefree: 0
   - Free: 11 (11)

I modify mkfs.f2fs forcedly as below, which could let generic/015 passed:

diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 8a821d3a1747..774bbef73d9a 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -371,6 +371,12 @@ static int f2fs_prepare_super_block(void)
(2 * (100 / c.overprovision + 1) + 6)
* c.segs_per_sec;

+   if (get_sb(segment_count_main) * (100 - c.overprovision) / 100 -
+   c.reserved_segments < 6) {
+   c.overprovision = 50;
+   c.reserved_segments = get_sb(segment_count_main) - 6 * 2;
+   }
+
uuid_generate(sb->uuid);

/* precompute checksum seed for metadata */

Thanks,


On 2017/10/16 11:25, Chao Yu wrote:

On 2017/10/14 20:34, Yunlong Song wrote:

Do you mean check out-of-space test? I have tried that but no bugon.

Yes, test recent f2fs codes with kernel 4.13.0-rc1+ in VM, FYI:

kernel BUG at gc.c:1034!
invalid opcode:  [#1] SMP
Hardware name: Xen HVM domU, BIOS 4.1.2_115-900.260_ 11/06/2015
RIP: 0010:f2fs_gc+0x6e5/0x6f0 [f2fs]
RSP: 0018:c90004af7b40 EFLAGS: 00010202
RAX: 8801b0a15940 RBX:  RCX: 
RDX: 8801b0a15940 RSI: 8801978d5f00 RDI: 880128148048
RBP: c90004af7c38 R08: 8801978d5f00 R09: 0003
R10: 0003 R11: 8800060703a0 R12: 
R13:  R14: 0001 R15: 8801b4279800
FS:  7f23493cb740() GS:880216f0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7ffd05402ff8 CR3: 0001bffb3000 CR4: 001406e0
Call Trace:
   f2fs_balance_fs+0x123/0x140 [f2fs]
   f2fs_create+0x130/0x240 [f2fs]
   path_openat+0xee7/0x1360
   do_filp_open+0x7e/0xd0
   do_sys_open+0x115/0x1f0
   SyS_open+0x1e/0x20
   do_syscall_64+0x6e/0x160
   entry_SYSCALL64_slow_path+0x25/0x25

Thanks,


On 2017/10/14 8:17, Chao Yu wrote:

On 2017/10/13 21:31, Yunlong Song wrote:

This can help us to debug on some corner case.

I can hit this bugon with generic/015 of fstest easily, could have a look at
this?

Thanks,


Signed-off-by: Yunlong Song 
Signed-off-by: Chao Yu 
---
fs/f2fs/gc.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 197ebf4..2b03202 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -986,6 +986,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
.ilist = LIST_HEAD_INIT(gc_list.ilist),
.iroot = RADIX_TREE_INIT(GFP_NOFS),
};
+   bool need_fggc = false;

	trace_f2fs_gc_begin(sbi->sb, sync, background,

get_pages(sbi, F2FS_DIRTY_NODES),
@@ -1018,8 +1019,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
if (ret)
goto stop;
}
-   if (has_not_enough_free_secs(sbi, 0, 0))
+   if (has_not_enough_free_secs(sbi, 0, 0)) {
gc_type = FG_GC;
+   need_fggc = true;
+   }
}

	/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */

@@ -1028,6 +1031,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
goto stop;
}
if (!__get_victim(sbi, , gc_type)) {
+   f2fs_bug_on(sbi, !total_freed && need_fggc);
ret = -ENODATA;
goto stop;
}


.


.



.



--
Thanks,
Yunlong Song

[PATCH v2] mtd: nand: Fix writing mtdoops to nand flash.

[motobud]

From: Brent Taylor 

When mtdoops calls mtd_panic_write, it eventually calls
panic_nand_write in nand_base.c.  In order to properly
wait for the nand chip to be ready in panic_nand_wait,
the chip must first be selected.

When using the atmel nand flash controller, a panic
would occur due to a NULL pointer exception.

Signed-off-by: Brent Taylor 
---
Changes in v2:
  - drop deselecting the chip
  - move select_chip and panic_nand_wait after panic_nand_get_device

 drivers/mtd/nand/nand_base.c | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 12edaae17d81..2093667e2bcb 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2802,12 +2802,16 @@ static int panic_nand_write(struct mtd_info *mtd, 
loff_t to, size_t len,
struct mtd_oob_ops ops;
int ret;
 
-   /* Wait for the device to get ready */
-   panic_nand_wait(mtd, chip, 400);
+   int chipnr = (int)(to >> chip->chip_shift);
 
/* Grab the device */
panic_nand_get_device(chip, mtd, FL_WRITING);
 
+   chip->select_chip(mtd, chipnr);
+
+   /* Wait for the device to get ready */
+   panic_nand_wait(mtd, chip, 400);
+
memset(, 0, sizeof(ops));
ops.len = len;
ops.datbuf = (uint8_t *)buf;
-- 
2.14.2

Re: [f2fs-dev] [PATCH v2] f2fs: add bug_on when f2fs_gc even fails to get one victim

[Yunlong Song]

So it seems it is really useful to add this bug_on in gc.

On 2017/10/31 11:17, Chao Yu wrote:

On 2017/10/31 9:32, Yunlong Song wrote:

I think there may be bugs somewhere, since no victim is selected but it
really needs gc.
What is the size of the data image?

I have providered the testcase, could you check that?


I can hit this bugon with generic/015 of fstest easily, could have a look at
this?

Anyway, I have looked into this issue at last weekend, forgot to post related
info. It shows that if the image size is very small, e.g. 50MB, current
segment will encroach overprov and reserved segments, so free segment count will
always be small than reserved segment threshold, result in triggering bugon in
FGGC.

[SB: 1] [CP: 2] [SIT: 2] [NAT: 2] [SSA: 1] [MAIN: 17(OverProv:14 Resv:12)]

   - Valid: 6
   - Dirty: 0
   - Prefree: 0
   - Free: 11 (11)

I modify mkfs.f2fs forcedly as below, which could let generic/015 passed:

diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 8a821d3a1747..774bbef73d9a 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -371,6 +371,12 @@ static int f2fs_prepare_super_block(void)
(2 * (100 / c.overprovision + 1) + 6)
* c.segs_per_sec;

+   if (get_sb(segment_count_main) * (100 - c.overprovision) / 100 -
+   c.reserved_segments < 6) {
+   c.overprovision = 50;
+   c.reserved_segments = get_sb(segment_count_main) - 6 * 2;
+   }
+
uuid_generate(sb->uuid);

/* precompute checksum seed for metadata */

Thanks,


On 2017/10/16 11:25, Chao Yu wrote:

On 2017/10/14 20:34, Yunlong Song wrote:

Do you mean check out-of-space test? I have tried that but no bugon.

Yes, test recent f2fs codes with kernel 4.13.0-rc1+ in VM, FYI:

kernel BUG at gc.c:1034!
invalid opcode:  [#1] SMP
Hardware name: Xen HVM domU, BIOS 4.1.2_115-900.260_ 11/06/2015
RIP: 0010:f2fs_gc+0x6e5/0x6f0 [f2fs]
RSP: 0018:c90004af7b40 EFLAGS: 00010202
RAX: 8801b0a15940 RBX:  RCX: 
RDX: 8801b0a15940 RSI: 8801978d5f00 RDI: 880128148048
RBP: c90004af7c38 R08: 8801978d5f00 R09: 0003
R10: 0003 R11: 8800060703a0 R12: 
R13:  R14: 0001 R15: 8801b4279800
FS:  7f23493cb740() GS:880216f0() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 7ffd05402ff8 CR3: 0001bffb3000 CR4: 001406e0
Call Trace:
   f2fs_balance_fs+0x123/0x140 [f2fs]
   f2fs_create+0x130/0x240 [f2fs]
   path_openat+0xee7/0x1360
   do_filp_open+0x7e/0xd0
   do_sys_open+0x115/0x1f0
   SyS_open+0x1e/0x20
   do_syscall_64+0x6e/0x160
   entry_SYSCALL64_slow_path+0x25/0x25

Thanks,


On 2017/10/14 8:17, Chao Yu wrote:

On 2017/10/13 21:31, Yunlong Song wrote:

This can help us to debug on some corner case.

I can hit this bugon with generic/015 of fstest easily, could have a look at
this?

Thanks,


Signed-off-by: Yunlong Song 
Signed-off-by: Chao Yu 
---
fs/f2fs/gc.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index 197ebf4..2b03202 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -986,6 +986,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
.ilist = LIST_HEAD_INIT(gc_list.ilist),
.iroot = RADIX_TREE_INIT(GFP_NOFS),
};
+   bool need_fggc = false;

	trace_f2fs_gc_begin(sbi->sb, sync, background,

get_pages(sbi, F2FS_DIRTY_NODES),
@@ -1018,8 +1019,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
if (ret)
goto stop;
}
-   if (has_not_enough_free_secs(sbi, 0, 0))
+   if (has_not_enough_free_secs(sbi, 0, 0)) {
gc_type = FG_GC;
+   need_fggc = true;
+   }
}

	/* f2fs_balance_fs doesn't need to do BG_GC in critical path. */

@@ -1028,6 +1031,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
goto stop;
}
if (!__get_victim(sbi, , gc_type)) {
+   f2fs_bug_on(sbi, !total_freed && need_fggc);
ret = -ENODATA;
goto stop;
}


.


.



.



--
Thanks,
Yunlong Song

Re: [f2fs-dev] [PATCH v2] f2fs: add bug_on when f2fs_gc even fails to get one victim

[Chao Yu]

On 2017/10/31 9:32, Yunlong Song wrote:
> I think there may be bugs somewhere, since no victim is selected but it 
> really needs gc.
> What is the size of the data image?

I have providered the testcase, could you check that?

 I can hit this bugon with generic/015 of fstest easily, could have a look 
 at
 this?

Anyway, I have looked into this issue at last weekend, forgot to post related
info. It shows that if the image size is very small, e.g. 50MB, current
segment will encroach overprov and reserved segments, so free segment count will
always be small than reserved segment threshold, result in triggering bugon in
FGGC.

[SB: 1] [CP: 2] [SIT: 2] [NAT: 2] [SSA: 1] [MAIN: 17(OverProv:14 Resv:12)]

  - Valid: 6
  - Dirty: 0
  - Prefree: 0
  - Free: 11 (11)

I modify mkfs.f2fs forcedly as below, which could let generic/015 passed:

diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 8a821d3a1747..774bbef73d9a 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -371,6 +371,12 @@ static int f2fs_prepare_super_block(void)
(2 * (100 / c.overprovision + 1) + 6)
* c.segs_per_sec;

+   if (get_sb(segment_count_main) * (100 - c.overprovision) / 100 -
+   c.reserved_segments < 6) {
+   c.overprovision = 50;
+   c.reserved_segments = get_sb(segment_count_main) - 6 * 2;
+   }
+
uuid_generate(sb->uuid);

/* precompute checksum seed for metadata */

Thanks,

> 
> On 2017/10/16 11:25, Chao Yu wrote:
>> On 2017/10/14 20:34, Yunlong Song wrote:
>>> Do you mean check out-of-space test? I have tried that but no bugon.
>> Yes, test recent f2fs codes with kernel 4.13.0-rc1+ in VM, FYI:
>>
>> kernel BUG at gc.c:1034!
>> invalid opcode:  [#1] SMP
>> Hardware name: Xen HVM domU, BIOS 4.1.2_115-900.260_ 11/06/2015
>> RIP: 0010:f2fs_gc+0x6e5/0x6f0 [f2fs]
>> RSP: 0018:c90004af7b40 EFLAGS: 00010202
>> RAX: 8801b0a15940 RBX:  RCX: 
>> RDX: 8801b0a15940 RSI: 8801978d5f00 RDI: 880128148048
>> RBP: c90004af7c38 R08: 8801978d5f00 R09: 0003
>> R10: 0003 R11: 8800060703a0 R12: 
>> R13:  R14: 0001 R15: 8801b4279800
>> FS:  7f23493cb740() GS:880216f0() knlGS:
>> CS:  0010 DS:  ES:  CR0: 80050033
>> CR2: 7ffd05402ff8 CR3: 0001bffb3000 CR4: 001406e0
>> Call Trace:
>>   f2fs_balance_fs+0x123/0x140 [f2fs]
>>   f2fs_create+0x130/0x240 [f2fs]
>>   path_openat+0xee7/0x1360
>>   do_filp_open+0x7e/0xd0
>>   do_sys_open+0x115/0x1f0
>>   SyS_open+0x1e/0x20
>>   do_syscall_64+0x6e/0x160
>>   entry_SYSCALL64_slow_path+0x25/0x25
>>
>> Thanks,
>>
>>> On 2017/10/14 8:17, Chao Yu wrote:
 On 2017/10/13 21:31, Yunlong Song wrote:
> This can help us to debug on some corner case.
 I can hit this bugon with generic/015 of fstest easily, could have a look 
 at
 this?

 Thanks,

> Signed-off-by: Yunlong Song 
> Signed-off-by: Chao Yu 
> ---
>fs/f2fs/gc.c | 6 +-
>1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 197ebf4..2b03202 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -986,6 +986,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   .ilist = LIST_HEAD_INIT(gc_list.ilist),
>   .iroot = RADIX_TREE_INIT(GFP_NOFS),
>   };
> + bool need_fggc = false;
>
>   trace_f2fs_gc_begin(sbi->sb, sync, background,
>   get_pages(sbi, F2FS_DIRTY_NODES),
> @@ -1018,8 +1019,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   if (ret)
>   goto stop;
>   }
> - if (has_not_enough_free_secs(sbi, 0, 0))
> + if (has_not_enough_free_secs(sbi, 0, 0)) {
>   gc_type = FG_GC;
> + need_fggc = true;
> + }
>   }
>
>   /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> @@ -1028,6 +1031,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   goto stop;
>   }
>   if (!__get_victim(sbi, , gc_type)) {
> + f2fs_bug_on(sbi, !total_freed && need_fggc);
>   ret = -ENODATA;
>   goto stop;
>   }
>
 .

>>
>> .
>>
> 

Re: [f2fs-dev] [PATCH v2] f2fs: add bug_on when f2fs_gc even fails to get one victim

[Chao Yu]

On 2017/10/31 9:32, Yunlong Song wrote:
> I think there may be bugs somewhere, since no victim is selected but it 
> really needs gc.
> What is the size of the data image?

I have providered the testcase, could you check that?

 I can hit this bugon with generic/015 of fstest easily, could have a look 
 at
 this?

Anyway, I have looked into this issue at last weekend, forgot to post related
info. It shows that if the image size is very small, e.g. 50MB, current
segment will encroach overprov and reserved segments, so free segment count will
always be small than reserved segment threshold, result in triggering bugon in
FGGC.

[SB: 1] [CP: 2] [SIT: 2] [NAT: 2] [SSA: 1] [MAIN: 17(OverProv:14 Resv:12)]

  - Valid: 6
  - Dirty: 0
  - Prefree: 0
  - Free: 11 (11)

I modify mkfs.f2fs forcedly as below, which could let generic/015 passed:

diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 8a821d3a1747..774bbef73d9a 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -371,6 +371,12 @@ static int f2fs_prepare_super_block(void)
(2 * (100 / c.overprovision + 1) + 6)
* c.segs_per_sec;

+   if (get_sb(segment_count_main) * (100 - c.overprovision) / 100 -
+   c.reserved_segments < 6) {
+   c.overprovision = 50;
+   c.reserved_segments = get_sb(segment_count_main) - 6 * 2;
+   }
+
uuid_generate(sb->uuid);

/* precompute checksum seed for metadata */

Thanks,

> 
> On 2017/10/16 11:25, Chao Yu wrote:
>> On 2017/10/14 20:34, Yunlong Song wrote:
>>> Do you mean check out-of-space test? I have tried that but no bugon.
>> Yes, test recent f2fs codes with kernel 4.13.0-rc1+ in VM, FYI:
>>
>> kernel BUG at gc.c:1034!
>> invalid opcode:  [#1] SMP
>> Hardware name: Xen HVM domU, BIOS 4.1.2_115-900.260_ 11/06/2015
>> RIP: 0010:f2fs_gc+0x6e5/0x6f0 [f2fs]
>> RSP: 0018:c90004af7b40 EFLAGS: 00010202
>> RAX: 8801b0a15940 RBX:  RCX: 
>> RDX: 8801b0a15940 RSI: 8801978d5f00 RDI: 880128148048
>> RBP: c90004af7c38 R08: 8801978d5f00 R09: 0003
>> R10: 0003 R11: 8800060703a0 R12: 
>> R13:  R14: 0001 R15: 8801b4279800
>> FS:  7f23493cb740() GS:880216f0() knlGS:
>> CS:  0010 DS:  ES:  CR0: 80050033
>> CR2: 7ffd05402ff8 CR3: 0001bffb3000 CR4: 001406e0
>> Call Trace:
>>   f2fs_balance_fs+0x123/0x140 [f2fs]
>>   f2fs_create+0x130/0x240 [f2fs]
>>   path_openat+0xee7/0x1360
>>   do_filp_open+0x7e/0xd0
>>   do_sys_open+0x115/0x1f0
>>   SyS_open+0x1e/0x20
>>   do_syscall_64+0x6e/0x160
>>   entry_SYSCALL64_slow_path+0x25/0x25
>>
>> Thanks,
>>
>>> On 2017/10/14 8:17, Chao Yu wrote:
 On 2017/10/13 21:31, Yunlong Song wrote:
> This can help us to debug on some corner case.
 I can hit this bugon with generic/015 of fstest easily, could have a look 
 at
 this?

 Thanks,

> Signed-off-by: Yunlong Song 
> Signed-off-by: Chao Yu 
> ---
>fs/f2fs/gc.c | 6 +-
>1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 197ebf4..2b03202 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -986,6 +986,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   .ilist = LIST_HEAD_INIT(gc_list.ilist),
>   .iroot = RADIX_TREE_INIT(GFP_NOFS),
>   };
> + bool need_fggc = false;
>
>   trace_f2fs_gc_begin(sbi->sb, sync, background,
>   get_pages(sbi, F2FS_DIRTY_NODES),
> @@ -1018,8 +1019,10 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   if (ret)
>   goto stop;
>   }
> - if (has_not_enough_free_secs(sbi, 0, 0))
> + if (has_not_enough_free_secs(sbi, 0, 0)) {
>   gc_type = FG_GC;
> + need_fggc = true;
> + }
>   }
>
>   /* f2fs_balance_fs doesn't need to do BG_GC in critical path. */
> @@ -1028,6 +1031,7 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>   goto stop;
>   }
>   if (!__get_victim(sbi, , gc_type)) {
> + f2fs_bug_on(sbi, !total_freed && need_fggc);
>   ret = -ENODATA;
>   goto stop;
>   }
>
 .

>>
>> .
>>
> 

[PATCH v7] housekeeping: Document isolcpus flags

[Frederic Weisbecker]

Document the latest updates on the isolcpus boot option. While at it,
let's also fix the details about the preferred way to isolate a set of
CPUs from the scheduler general domains. Cpusets offer a much better
interface to achieve that.

Signed-off-by: Frederic Weisbecker 
Cc: Chris Metcalf 
Cc: Rik van Riel 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Mike Galbraith 
Cc: Ingo Molnar 
Cc: Christoph Lameter 
Cc: Paul E. McKenney 
Cc: Wanpeng Li 
Cc: Luiz Capitulino 
---
 Documentation/admin-guide/kernel-parameters.txt | 40 -
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 6b99c8b..62bc8b5 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1727,20 +1727,32 @@
isapnp= [ISAPNP]
Format: ,,,
 
-   isolcpus=   [KNL,SMP] Isolate CPUs from the general scheduler.
-   The argument is a cpu list, as described above.
-
-   This option can be used to specify one or more CPUs
-   to isolate from the general SMP balancing and scheduling
-   algorithms. You can move a process onto or off an
-   "isolated" CPU via the CPU affinity syscalls or cpuset.
-begins at 0 and the maximum value is
-   "number of CPUs in system - 1".
-
-   This option is the preferred way to isolate CPUs. The
-   alternative -- manually setting the CPU mask of all
-   tasks in the system -- can cause problems and
-   suboptimal load balancer performance.
+   isolcpus=   [KNL,SMP] Isolate a given set of CPUs from disturbance.
+   Format: [flag-list,]
+
+   Specify one or more CPUs to isolate from disturbances
+   specified in the flag list (default: domain):
+
+   nohz
+ Disable the tick when a single task runs.
+   domain
+ Isolate from the general SMP balancing and scheduling
+ algorithms. Note that performing domain isolation 
this way
+ is irreversible. It's not possible to bring back a 
CPU to
+ the domains once isolated through isolcpus. It's 
strongly
+ advised to use Cpusets instead to disable scheduler 
load
+ balancing through the file 
"cpuset.sched_load_balance".
+ It offers a much more flexible interface where CPUs 
can
+ move in and out of an isolated set anytime.
+
+ You can move a process onto or off an "isolated" CPU 
via
+ the CPU affinity syscalls or cpuset.
+  begins at 0 and the maximum value is
+ "number of CPUs in system - 1".
+
+   The format of  is described above.
+
+
 
iucv=   [HW,NET]
 
-- 
2.7.4

[PATCH v7] housekeeping: Document isolcpus flags

[Frederic Weisbecker]

Document the latest updates on the isolcpus boot option. While at it,
let's also fix the details about the preferred way to isolate a set of
CPUs from the scheduler general domains. Cpusets offer a much better
interface to achieve that.

Signed-off-by: Frederic Weisbecker 
Cc: Chris Metcalf 
Cc: Rik van Riel 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: Mike Galbraith 
Cc: Ingo Molnar 
Cc: Christoph Lameter 
Cc: Paul E. McKenney 
Cc: Wanpeng Li 
Cc: Luiz Capitulino 
---
 Documentation/admin-guide/kernel-parameters.txt | 40 -
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt 
b/Documentation/admin-guide/kernel-parameters.txt
index 6b99c8b..62bc8b5 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1727,20 +1727,32 @@
isapnp= [ISAPNP]
Format: ,,,
 
-   isolcpus=   [KNL,SMP] Isolate CPUs from the general scheduler.
-   The argument is a cpu list, as described above.
-
-   This option can be used to specify one or more CPUs
-   to isolate from the general SMP balancing and scheduling
-   algorithms. You can move a process onto or off an
-   "isolated" CPU via the CPU affinity syscalls or cpuset.
-begins at 0 and the maximum value is
-   "number of CPUs in system - 1".
-
-   This option is the preferred way to isolate CPUs. The
-   alternative -- manually setting the CPU mask of all
-   tasks in the system -- can cause problems and
-   suboptimal load balancer performance.
+   isolcpus=   [KNL,SMP] Isolate a given set of CPUs from disturbance.
+   Format: [flag-list,]
+
+   Specify one or more CPUs to isolate from disturbances
+   specified in the flag list (default: domain):
+
+   nohz
+ Disable the tick when a single task runs.
+   domain
+ Isolate from the general SMP balancing and scheduling
+ algorithms. Note that performing domain isolation 
this way
+ is irreversible. It's not possible to bring back a 
CPU to
+ the domains once isolated through isolcpus. It's 
strongly
+ advised to use Cpusets instead to disable scheduler 
load
+ balancing through the file 
"cpuset.sched_load_balance".
+ It offers a much more flexible interface where CPUs 
can
+ move in and out of an isolated set anytime.
+
+ You can move a process onto or off an "isolated" CPU 
via
+ the CPU affinity syscalls or cpuset.
+  begins at 0 and the maximum value is
+ "number of CPUs in system - 1".
+
+   The format of  is described above.
+
+
 
iucv=   [HW,NET]
 
-- 
2.7.4

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

[Chris Packham]

Hi Miquel,

On 09/10/17 19:19, Miquel RAYNAL wrote:
> Hello Kalyan,
> 
> On Mon, 9 Oct 2017 02:31:30 +
> Kalyan Kinthada  wrote:
> 
>> On 05/10/17 20:41, Miquel RAYNAL wrote:
>>> Hello Kalyan,
>>>
>>> On Thu, 28 Sep 2017 13:57:56 +1300
>>> Kalyan Kinthada  wrote:
>>>   
 When the arbitration between NOR and NAND flash is enabled
 the  field bit[21] in the Data Flash Control Register
 needs to be set to 1 according to guidleine GL-5830741 mentioned
>>> Typo: guideline ^
>> I will correct this typo in the next patch version.
 in Marvell Errata document MV-S501377-00, Rev. D.
>>> Thanks for that, I checked it.
>>>   
 This commit sets the FORCE_CSX bit to 1 for all
 ARMADA370 variants as the arbitration is always enabled by default.
 This change does not apply for pxa3xx variants because the
 FORCE_CSX bit does not exist/reserved on the NFCv1.
>>> Sorry to bother you again but I am reworking the pxa3xx_nand driver
>>> and so I would like to deeply understand why this is needed because
>>> I will have to integrate it in my work too.
>>>
>>> So please tell me what is your setup, do you really use NAND/NOR
>>> arbitration? Do you have not-Don't Care CS NAND chips? I have some
>>> doubts because even if the spec precises in the NAND controller
>>> chapter that arbitration is always enabled by default, the bit 27
>>> (NfArbiterEn) in the SoC Device Multiplex Register at offset
>>> 0x00018208 is actually at 0 by default (disabled). Did you enable
>>> this bit manually ? I checked with devmem on my setup and this bit
>>> was unset.
>> Yes, my setup use NAND/NOR arbitration and use a Don't
>> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
>> by default in my case. I did not manually set the bit 27 to 1.
> 
> Actually if you use Don't Care CS NAND chips you should not need the
> force CS bit, as it is mentioned in the guidelines you pointed, this
> bit is only useful for *not* don't care CS NAND chips (and the name
> "force CS" is pretty straight forward too!).

It's less straight forward than you think. It's FORCE_CSX (the X is 
important) which is defined as "force chip select false on busy". What 
it means is that the NF_CSn is de-asserted if the interface is 
arbitrated away from the NAND flash controller. I think that means you 
won't end up with the chip-select for the dev-bus/NOR firing immediately 
when that device is arbitrated.

> Otherwise what bootloader and kernel do you use to have this
> bit set by default? You may also want to check if this bit is set by
> your bootloader by stopping it before it loads Linux and check manually
> the value of this bit with 'md' (on U-Boot).
> 
> Thank you,
> Miquèl
> 
>>
>> Please let me now if you have any other question or do you want me
>> to send the updated patch version with the typo fixed.
>>
>> Thank You,
>> Kalyan
>>
>>>
>>> Thank you for your time,
>>> Miquèl
>>>   
 The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
 unused so remove it along with NDCR_NAND_MODE.

 Signed-off-by: Kalyan Kinthada
  ---
>>>
>>>   
drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/drivers/mtd/nand/pxa3xx_nand.c
 b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
 +++ b/drivers/mtd/nand/pxa3xx_nand.c
 @@ -67,8 +67,7 @@
#define NDCR_DWIDTH_M   (0x1 << 26)
#define NDCR_PAGE_SZ(0x1 << 24)
#define NDCR_NCSX   (0x1 << 23)
 -#define NDCR_ND_MODE  (0x3 << 21)
 -#define NDCR_NAND_MODE(0x0)
 +#define NDCR_FORCE_CSX(0x1 << 21)
#define NDCR_CLR_PG_CNT (0x1 << 20)
#define NFCV1_NDCR_ARB_CNTL (0x1 << 19)
#define NFCV2_NDCR_STOP_ON_UNCOR(0x1 << 19)
 @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
 pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;

 @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
 pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
 NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if 

Re: [PATCH v3 1/1] mtd: nand: pxa3xx: Set FORCE_CSX bit to ARMADA370 variants.

[Chris Packham]

Hi Miquel,

On 09/10/17 19:19, Miquel RAYNAL wrote:
> Hello Kalyan,
> 
> On Mon, 9 Oct 2017 02:31:30 +
> Kalyan Kinthada  wrote:
> 
>> On 05/10/17 20:41, Miquel RAYNAL wrote:
>>> Hello Kalyan,
>>>
>>> On Thu, 28 Sep 2017 13:57:56 +1300
>>> Kalyan Kinthada  wrote:
>>>   
 When the arbitration between NOR and NAND flash is enabled
 the  field bit[21] in the Data Flash Control Register
 needs to be set to 1 according to guidleine GL-5830741 mentioned
>>> Typo: guideline ^
>> I will correct this typo in the next patch version.
 in Marvell Errata document MV-S501377-00, Rev. D.
>>> Thanks for that, I checked it.
>>>   
 This commit sets the FORCE_CSX bit to 1 for all
 ARMADA370 variants as the arbitration is always enabled by default.
 This change does not apply for pxa3xx variants because the
 FORCE_CSX bit does not exist/reserved on the NFCv1.
>>> Sorry to bother you again but I am reworking the pxa3xx_nand driver
>>> and so I would like to deeply understand why this is needed because
>>> I will have to integrate it in my work too.
>>>
>>> So please tell me what is your setup, do you really use NAND/NOR
>>> arbitration? Do you have not-Don't Care CS NAND chips? I have some
>>> doubts because even if the spec precises in the NAND controller
>>> chapter that arbitration is always enabled by default, the bit 27
>>> (NfArbiterEn) in the SoC Device Multiplex Register at offset
>>> 0x00018208 is actually at 0 by default (disabled). Did you enable
>>> this bit manually ? I checked with devmem on my setup and this bit
>>> was unset.
>> Yes, my setup use NAND/NOR arbitration and use a Don't
>> care CS Nand Chip. The bit 27 at offset 0x00018208 is set to 1
>> by default in my case. I did not manually set the bit 27 to 1.
> 
> Actually if you use Don't Care CS NAND chips you should not need the
> force CS bit, as it is mentioned in the guidelines you pointed, this
> bit is only useful for *not* don't care CS NAND chips (and the name
> "force CS" is pretty straight forward too!).

It's less straight forward than you think. It's FORCE_CSX (the X is 
important) which is defined as "force chip select false on busy". What 
it means is that the NF_CSn is de-asserted if the interface is 
arbitrated away from the NAND flash controller. I think that means you 
won't end up with the chip-select for the dev-bus/NOR firing immediately 
when that device is arbitrated.

> Otherwise what bootloader and kernel do you use to have this
> bit set by default? You may also want to check if this bit is set by
> your bootloader by stopping it before it loads Linux and check manually
> the value of this bit with 'md' (on U-Boot).
> 
> Thank you,
> Miquèl
> 
>>
>> Please let me now if you have any other question or do you want me
>> to send the updated patch version with the typo fixed.
>>
>> Thank You,
>> Kalyan
>>
>>>
>>> Thank you for your time,
>>> Miquèl
>>>   
 The NDCR_ND_MODE conflicts with the new NDCR_FORCE_CSX but is
 unused so remove it along with NDCR_NAND_MODE.

 Signed-off-by: Kalyan Kinthada
  ---
>>>
>>>   
drivers/mtd/nand/pxa3xx_nand.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)

 diff --git a/drivers/mtd/nand/pxa3xx_nand.c
 b/drivers/mtd/nand/pxa3xx_nand.c index 85cff68643e0..a6a7a5af7bed
 100644 --- a/drivers/mtd/nand/pxa3xx_nand.c
 +++ b/drivers/mtd/nand/pxa3xx_nand.c
 @@ -67,8 +67,7 @@
#define NDCR_DWIDTH_M   (0x1 << 26)
#define NDCR_PAGE_SZ(0x1 << 24)
#define NDCR_NCSX   (0x1 << 23)
 -#define NDCR_ND_MODE  (0x3 << 21)
 -#define NDCR_NAND_MODE(0x0)
 +#define NDCR_FORCE_CSX(0x1 << 21)
#define NDCR_CLR_PG_CNT (0x1 << 20)
#define NFCV1_NDCR_ARB_CNTL (0x1 << 19)
#define NFCV2_NDCR_STOP_ON_UNCOR(0x1 << 19)
 @@ -1464,6 +1463,9 @@ static int pxa3xx_nand_config_ident(struct
 pxa3xx_nand_info *info) info->chunk_size = PAGE_CHUNK_SIZE;
info->reg_ndcr = 0x0; /* enable all interrupts */
info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;
info->reg_ndcr |= NDCR_RD_ID_CNT(READ_ID_BYTES);
info->reg_ndcr |= NDCR_SPARE_EN;

 @@ -1498,6 +1500,9 @@ static void pxa3xx_nand_detect_config(struct
 pxa3xx_nand_info *info) info->reg_ndcr = ndcr &
~(NDCR_INT_MASK | NDCR_ND_ARB_EN |
 NFCV1_NDCR_ARB_CNTL); info->reg_ndcr |= (pdata->enable_arbiter) ?
 NDCR_ND_ARB_EN : 0;
 +  /* Set FORCE_CSX bit for all ARMADA370 Variants. Ref#:
 GL-5830741 */
 +  if (info->variant == PXA3XX_NAND_VARIANT_ARMADA370)
 +  info->reg_ndcr |= NDCR_FORCE_CSX;

Re: [Xen-devel] [PATCH v5 1/1] xen/time: do not decrease steal time after live migration on xen

[Dongli Zhang]

Hi Boris,

On 10/31/2017 08:58 AM, Boris Ostrovsky wrote:
> 
> 
> On 10/30/2017 08:14 PM, Dongli Zhang wrote:
>> Hi Boris,
>>
>> On 10/30/2017 09:34 PM, Boris Ostrovsky wrote:
>>> On 10/30/2017 04:03 AM, Dongli Zhang wrote:
 After guest live migration on xen, steal time in /proc/stat
 (cpustat[CPUTIME_STEAL]) might decrease because steal returned by
 xen_steal_lock() might be less than this_rq()->prev_steal_time which is
 derived from previous return value of xen_steal_clock().

 For instance, steal time of each vcpu is 335 before live migration.

 cpu  198 0 368 200064 1962 0 0 1340 0 0
 cpu0 38 0 81 50063 492 0 0 335 0 0
 cpu1 65 0 97 49763 634 0 0 335 0 0
 cpu2 38 0 81 50098 462 0 0 335 0 0
 cpu3 56 0 107 50138 374 0 0 335 0 0

 After live migration, steal time is reduced to 312.

 cpu  200 0 370 200330 1971 0 0 1248 0 0
 cpu0 38 0 82 50123 500 0 0 312 0 0
 cpu1 65 0 97 49832 634 0 0 312 0 0
 cpu2 39 0 82 50167 462 0 0 312 0 0
 cpu3 56 0 107 50207 374 0 0 312 0 0

 Since runstate times are cumulative and cleared during xen live migration
 by xen hypervisor, the idea of this patch is to accumulate runstate times
 to global percpu variables before live migration suspend. Once guest VM is
 resumed, xen_get_runstate_snapshot_cpu() would always return the sum of new
 runstate times and previously accumulated times stored in global percpu
 variables.

 Similar and more severe issue would impact prior linux 4.8-4.10 as
 discussed by Michael Las at
 https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest,

 which would overflow steal time and lead to 100% st usage in top command
 for linux 4.8-4.10. A backport of this patch would fix that issue.

 References:
 https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest

 Signed-off-by: Dongli Zhang 

 ---
 Changed since v1:
* relocate modification to xen_get_runstate_snapshot_cpu

 Changed since v2:
* accumulate runstate times before live migration

 Changed since v3:
* do not accumulate times in the case of guest checkpointing

 Changed since v4:
* allocate array of vcpu_runstate_info to reduce number of memory 
 allocation

 ---
   drivers/xen/manage.c |  2 ++
   drivers/xen/time.c   | 68
 ++--
   include/xen/interface/vcpu.h |  2 ++
   include/xen/xen-ops.h|  1 +
   4 files changed, 71 insertions(+), 2 deletions(-)

 diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
 index c425d03..3dc085d 100644
 --- a/drivers/xen/manage.c
 +++ b/drivers/xen/manage.c
 @@ -72,6 +72,7 @@ static int xen_suspend(void *data)
   }
 gnttab_suspend();
 +xen_accumulate_runstate_time(-1);
   xen_arch_pre_suspend();
 /*
 @@ -84,6 +85,7 @@ static int xen_suspend(void *data)
  : 0);
 xen_arch_post_suspend(si->cancelled);
 +xen_accumulate_runstate_time(si->cancelled);
>>>
>>> I am not convinced that the comment above HYPERVISOR_suspend() is
>>> correct. The call can return an error code and so if it returns -EPERM
>>> (which AFAICS it can't now but might in the future) then
>>> xen_accumulate_runstate_time() will do wrong thing.
>>
>> I would split xen_accumulate_runstate_time() into two functions to avoid the
>> -EPERM issue, as one is for saving and another is for accumulation, 
>> respectively.
>>
>> Otherwise, can you use xen_accumulate_runstate_time(2) for saving before 
>> suspend
>> and xen_accumulate_runstate_time(si->cancelled) after resume?
> 
> 
> I'd probably just say something like
> 
> si->cancelled = HYPERVISOR_suspend() ? 1 : 0;

As the call of HYPERVISOR_suspend() takes 3 lines, I would make it as below and
I think gcc would optimize it.

-   /*
-* This hypercall returns 1 if suspend was cancelled
-* or the domain was merely checkpointed, and 0 if it
-* is resuming in a new domain.
-*/
si->cancelled = HYPERVISOR_suspend(xen_pv_domain()
? virt_to_gfn(xen_start_info)
: 0);
+   si->cancelled = si->cancelled ? 1 : 0;

> 
> and keep xen_accumulate_runstate_time() as is (maybe rename it to
> xen_manage_runstate_time()). And also remove the comment above the hypercall 
> as
> it is incorrect (but please mention the reason in the commit message)
> 
>>
>>>
>>>
   gnttab_resume();
 if (!si->cancelled) {
 diff --git a/drivers/xen/time.c b/drivers/xen/time.c
 index ac5f23f..cf3afb9 100644
 --- a/drivers/xen/time.c
 +++ b/drivers/xen/time.c
 @@ -19,6 +19,9 @@
   /* runstate 

Re: [Xen-devel] [PATCH v5 1/1] xen/time: do not decrease steal time after live migration on xen

[Dongli Zhang]

Hi Boris,

On 10/31/2017 08:58 AM, Boris Ostrovsky wrote:
> 
> 
> On 10/30/2017 08:14 PM, Dongli Zhang wrote:
>> Hi Boris,
>>
>> On 10/30/2017 09:34 PM, Boris Ostrovsky wrote:
>>> On 10/30/2017 04:03 AM, Dongli Zhang wrote:
 After guest live migration on xen, steal time in /proc/stat
 (cpustat[CPUTIME_STEAL]) might decrease because steal returned by
 xen_steal_lock() might be less than this_rq()->prev_steal_time which is
 derived from previous return value of xen_steal_clock().

 For instance, steal time of each vcpu is 335 before live migration.

 cpu  198 0 368 200064 1962 0 0 1340 0 0
 cpu0 38 0 81 50063 492 0 0 335 0 0
 cpu1 65 0 97 49763 634 0 0 335 0 0
 cpu2 38 0 81 50098 462 0 0 335 0 0
 cpu3 56 0 107 50138 374 0 0 335 0 0

 After live migration, steal time is reduced to 312.

 cpu  200 0 370 200330 1971 0 0 1248 0 0
 cpu0 38 0 82 50123 500 0 0 312 0 0
 cpu1 65 0 97 49832 634 0 0 312 0 0
 cpu2 39 0 82 50167 462 0 0 312 0 0
 cpu3 56 0 107 50207 374 0 0 312 0 0

 Since runstate times are cumulative and cleared during xen live migration
 by xen hypervisor, the idea of this patch is to accumulate runstate times
 to global percpu variables before live migration suspend. Once guest VM is
 resumed, xen_get_runstate_snapshot_cpu() would always return the sum of new
 runstate times and previously accumulated times stored in global percpu
 variables.

 Similar and more severe issue would impact prior linux 4.8-4.10 as
 discussed by Michael Las at
 https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest,

 which would overflow steal time and lead to 100% st usage in top command
 for linux 4.8-4.10. A backport of this patch would fix that issue.

 References:
 https://0xstubs.org/debugging-a-flaky-cpu-steal-time-counter-on-a-paravirtualized-xen-guest

 Signed-off-by: Dongli Zhang 

 ---
 Changed since v1:
* relocate modification to xen_get_runstate_snapshot_cpu

 Changed since v2:
* accumulate runstate times before live migration

 Changed since v3:
* do not accumulate times in the case of guest checkpointing

 Changed since v4:
* allocate array of vcpu_runstate_info to reduce number of memory 
 allocation

 ---
   drivers/xen/manage.c |  2 ++
   drivers/xen/time.c   | 68
 ++--
   include/xen/interface/vcpu.h |  2 ++
   include/xen/xen-ops.h|  1 +
   4 files changed, 71 insertions(+), 2 deletions(-)

 diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
 index c425d03..3dc085d 100644
 --- a/drivers/xen/manage.c
 +++ b/drivers/xen/manage.c
 @@ -72,6 +72,7 @@ static int xen_suspend(void *data)
   }
 gnttab_suspend();
 +xen_accumulate_runstate_time(-1);
   xen_arch_pre_suspend();
 /*
 @@ -84,6 +85,7 @@ static int xen_suspend(void *data)
  : 0);
 xen_arch_post_suspend(si->cancelled);
 +xen_accumulate_runstate_time(si->cancelled);
>>>
>>> I am not convinced that the comment above HYPERVISOR_suspend() is
>>> correct. The call can return an error code and so if it returns -EPERM
>>> (which AFAICS it can't now but might in the future) then
>>> xen_accumulate_runstate_time() will do wrong thing.
>>
>> I would split xen_accumulate_runstate_time() into two functions to avoid the
>> -EPERM issue, as one is for saving and another is for accumulation, 
>> respectively.
>>
>> Otherwise, can you use xen_accumulate_runstate_time(2) for saving before 
>> suspend
>> and xen_accumulate_runstate_time(si->cancelled) after resume?
> 
> 
> I'd probably just say something like
> 
> si->cancelled = HYPERVISOR_suspend() ? 1 : 0;

As the call of HYPERVISOR_suspend() takes 3 lines, I would make it as below and
I think gcc would optimize it.

-   /*
-* This hypercall returns 1 if suspend was cancelled
-* or the domain was merely checkpointed, and 0 if it
-* is resuming in a new domain.
-*/
si->cancelled = HYPERVISOR_suspend(xen_pv_domain()
? virt_to_gfn(xen_start_info)
: 0);
+   si->cancelled = si->cancelled ? 1 : 0;

> 
> and keep xen_accumulate_runstate_time() as is (maybe rename it to
> xen_manage_runstate_time()). And also remove the comment above the hypercall 
> as
> it is incorrect (but please mention the reason in the commit message)
> 
>>
>>>
>>>
   gnttab_resume();
 if (!si->cancelled) {
 diff --git a/drivers/xen/time.c b/drivers/xen/time.c
 index ac5f23f..cf3afb9 100644
 --- a/drivers/xen/time.c
 +++ b/drivers/xen/time.c
 @@ -19,6 +19,9 @@
   /* runstate info updated by Xen */
 

Re: Kernel crash in free_pipe_info()

[Linus Torvalds]

On Mon, Oct 30, 2017 at 7:08 PM, Linus Torvalds
 wrote:
>
> I'm not seeing anything that makes sense. I'll have to think about this.

Al, would you mind taking a look at the error handling in create_pipe_files().

In particular, look here:

 - we start out allocating the inode with "get_pipe_inode().

   That sets up a inode->i_pipe, with pipe->files initialized to 2.
Fine. We're going to have two file descriptors.

 - we then create the dummy path:

path.dentry = d_alloc_pseudo(pipe_mnt->mnt_sb, _name);

   fine fine. Again, this looks all good for the success cases.

But the *error* cases are a bit dodgy, aren't they?

We have three different error cases:

 - we couldn't even allocate a dentry. We do

free_pipe_info(inode->i_pipe);
iput(inode);

 - we couldn't allocate any file at all:

free_pipe_info(inode->i_pipe);
path_put();

 - we allocated the first file, but not the second:

put_filp(f);
free_pipe_info(inode->i_pipe);
path_put();

and it worries me a bit that in all those error cases, we end up doing
that "free_pipe_info()", but we basically do this half-arsed job of
freeing things.

For example, we use "put_filp()" to free the file pointer, not "fput()".

We do that "free_pipe_info(inode->i_pipe);", but we never actually
clear inode->i_pipe, so now we have an inode that looks like a pipe
inode, and has a stale pointer to a pipe_inode_info.

It all looks technically correct. It's fine to use put_filp(), because
the file pointer has never really been used. And the inode should
never get re-used anyway without going through the whole reinit in
inode_init_always().

So I don't see anything *wrong*, but I see a lot that is just unusual,
and seems to depend on half-initialized state being fine. Can you look
at this?

Linus

Re: Kernel crash in free_pipe_info()

[Linus Torvalds]

On Mon, Oct 30, 2017 at 7:08 PM, Linus Torvalds
 wrote:
>
> I'm not seeing anything that makes sense. I'll have to think about this.

Al, would you mind taking a look at the error handling in create_pipe_files().

In particular, look here:

 - we start out allocating the inode with "get_pipe_inode().

   That sets up a inode->i_pipe, with pipe->files initialized to 2.
Fine. We're going to have two file descriptors.

 - we then create the dummy path:

path.dentry = d_alloc_pseudo(pipe_mnt->mnt_sb, _name);

   fine fine. Again, this looks all good for the success cases.

But the *error* cases are a bit dodgy, aren't they?

We have three different error cases:

 - we couldn't even allocate a dentry. We do

free_pipe_info(inode->i_pipe);
iput(inode);

 - we couldn't allocate any file at all:

free_pipe_info(inode->i_pipe);
path_put();

 - we allocated the first file, but not the second:

put_filp(f);
free_pipe_info(inode->i_pipe);
path_put();

and it worries me a bit that in all those error cases, we end up doing
that "free_pipe_info()", but we basically do this half-arsed job of
freeing things.

For example, we use "put_filp()" to free the file pointer, not "fput()".

We do that "free_pipe_info(inode->i_pipe);", but we never actually
clear inode->i_pipe, so now we have an inode that looks like a pipe
inode, and has a stale pointer to a pipe_inode_info.

It all looks technically correct. It's fine to use put_filp(), because
the file pointer has never really been used. And the inode should
never get re-used anyway without going through the whole reinit in
inode_init_always().

So I don't see anything *wrong*, but I see a lot that is just unusual,
and seems to depend on half-initialized state being fine. Can you look
at this?

Linus

RE: [PATCH v10 0/4] this patchset is to remove PPCisms for QEIC

[Qiang Zhao]

Hi all,

Could anybody review this patchset and take action on them? Thank you!

Best Regards
Qiang Zhao

> > -Original Message-
> > From: Zhao Qiang [mailto:qiang.z...@nxp.com]
> > Sent: Monday, August 07, 2017 11:07 AM
> > To: t...@linutronix.de
> > Cc: o...@buserror.net; Xiaobo Xie ; linux-
> > ker...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; Qiang Zhao
> > 
> > Subject: [PATCH v10 0/4] this patchset is to remove PPCisms for QEIC
> >
> > QEIC is supported more than just powerpc boards, so remove PPCisms.
> >
> > changelog:
> > Changes for v8:
> > - use IRQCHIP_DECLARE() instead of subsys_initcall in qeic driver
> > - remove include/soc/fsl/qe/qe_ic.h
> > Changes for v9:
> > - rebase
> > - fix the compile issue when apply the second patch, in fact, there
> > was no compile issue
> >   when apply all the patches of this patchset
> > Changes for v10:
> > - simplify codes, remove duplicated codes
> >
> > Zhao Qiang (4):
> >   irqchip/qeic: move qeic driver from drivers/soc/fsl/qe
> > Changes for v2:
> > - modify the subject and commit msg
> > Changes for v3:
> > - merge .h file to .c, rename it with irq-qeic.c
> > Changes for v4:
> > - modify comments
> > Changes for v5:
> > - disable rename detection
> > Changes for v6:
> > - rebase
> > Changes for v7:
> > - na
> >
> >   irqchip/qeic: merge qeic init code from platforms to a common function
> > Changes for v2:
> > - modify subject and commit msg
> > - add check for qeic by type
> > Changes for v3:
> > - na
> > Changes for v4:
> > - na
> > Changes for v5:
> > - na
> > Changes for v6:
> > - rebase
> > Changes for v7:
> > - na
> > Changes for v8:
> > - use IRQCHIP_DECLARE() instead of subsys_initcall
> >
> >   irqchip/qeic: merge qeic_of_init into qe_ic_init
> > Changes for v2:
> > - modify subject and commit msg
> > - return 0 and add put node when return in qe_ic_init
> > Changes for v3:
> > - na
> > Changes for v4:
> > - na
> > Changes for v5:
> > - na
> > Changes for v6:
> > - rebase
> > Changes for v7:
> > - na
> >
> >   irqchip/qeic: remove PPCisms for QEIC
> > Changes for v6:
> > - new added
> > Changes for v7:
> > - fix warning
> > Changes for v8:
> > - remove include/soc/fsl/qe/qe_ic.h
> >
> > Zhao Qiang (4):
> >   irqchip/qeic: move qeic driver from drivers/soc/fsl/qe
> >   irqchip/qeic: merge qeic init code from platforms to a common function
> >   irqchip/qeic: merge qeic_of_init into qe_ic_init
> >   irqchip/qeic: remove PPCisms for QEIC
> >
> >  MAINTAINERS|   6 +
> >  arch/powerpc/platforms/83xx/km83xx.c   |   1 -
> >  arch/powerpc/platforms/83xx/misc.c |  16 -
> >  arch/powerpc/platforms/83xx/mpc832x_mds.c  |   1 -
> >  arch/powerpc/platforms/83xx/mpc832x_rdb.c  |   1 -
> >  arch/powerpc/platforms/83xx/mpc836x_mds.c  |   1 -
> >  arch/powerpc/platforms/83xx/mpc836x_rdk.c  |   1 -
> >  arch/powerpc/platforms/85xx/corenet_generic.c  |  10 -
> >  arch/powerpc/platforms/85xx/mpc85xx_mds.c  |  15 -
> >  arch/powerpc/platforms/85xx/mpc85xx_rdb.c  |  17 -
> >  arch/powerpc/platforms/85xx/twr_p102x.c|  15 -
> >  drivers/irqchip/Makefile   |   1 +
> >  drivers/{soc/fsl/qe/qe_ic.c => irqchip/irq-qeic.c} | 358 
> > -
> >  drivers/soc/fsl/qe/Makefile|   2 +-
> >  drivers/soc/fsl/qe/qe_ic.h | 103 --
> >  include/soc/fsl/qe/qe_ic.h | 139 
> >  16 files changed, 218 insertions(+), 469 deletions(-)  rename
> > drivers/{soc/fsl/qe/qe_ic.c => irqchip/irq-qeic.c} (58%)  delete mode
> > 100644 drivers/soc/fsl/qe/qe_ic.h  delete mode 100644
> > include/soc/fsl/qe/qe_ic.h
> >
> > --
> > 2.1.0.27.g96db324

RE: [PATCH v10 0/4] this patchset is to remove PPCisms for QEIC

[Qiang Zhao]

Hi all,

Could anybody review this patchset and take action on them? Thank you!

Best Regards
Qiang Zhao

> > -Original Message-
> > From: Zhao Qiang [mailto:qiang.z...@nxp.com]
> > Sent: Monday, August 07, 2017 11:07 AM
> > To: t...@linutronix.de
> > Cc: o...@buserror.net; Xiaobo Xie ; linux-
> > ker...@vger.kernel.org; linuxppc-...@lists.ozlabs.org; Qiang Zhao
> > 
> > Subject: [PATCH v10 0/4] this patchset is to remove PPCisms for QEIC
> >
> > QEIC is supported more than just powerpc boards, so remove PPCisms.
> >
> > changelog:
> > Changes for v8:
> > - use IRQCHIP_DECLARE() instead of subsys_initcall in qeic driver
> > - remove include/soc/fsl/qe/qe_ic.h
> > Changes for v9:
> > - rebase
> > - fix the compile issue when apply the second patch, in fact, there
> > was no compile issue
> >   when apply all the patches of this patchset
> > Changes for v10:
> > - simplify codes, remove duplicated codes
> >
> > Zhao Qiang (4):
> >   irqchip/qeic: move qeic driver from drivers/soc/fsl/qe
> > Changes for v2:
> > - modify the subject and commit msg
> > Changes for v3:
> > - merge .h file to .c, rename it with irq-qeic.c
> > Changes for v4:
> > - modify comments
> > Changes for v5:
> > - disable rename detection
> > Changes for v6:
> > - rebase
> > Changes for v7:
> > - na
> >
> >   irqchip/qeic: merge qeic init code from platforms to a common function
> > Changes for v2:
> > - modify subject and commit msg
> > - add check for qeic by type
> > Changes for v3:
> > - na
> > Changes for v4:
> > - na
> > Changes for v5:
> > - na
> > Changes for v6:
> > - rebase
> > Changes for v7:
> > - na
> > Changes for v8:
> > - use IRQCHIP_DECLARE() instead of subsys_initcall
> >
> >   irqchip/qeic: merge qeic_of_init into qe_ic_init
> > Changes for v2:
> > - modify subject and commit msg
> > - return 0 and add put node when return in qe_ic_init
> > Changes for v3:
> > - na
> > Changes for v4:
> > - na
> > Changes for v5:
> > - na
> > Changes for v6:
> > - rebase
> > Changes for v7:
> > - na
> >
> >   irqchip/qeic: remove PPCisms for QEIC
> > Changes for v6:
> > - new added
> > Changes for v7:
> > - fix warning
> > Changes for v8:
> > - remove include/soc/fsl/qe/qe_ic.h
> >
> > Zhao Qiang (4):
> >   irqchip/qeic: move qeic driver from drivers/soc/fsl/qe
> >   irqchip/qeic: merge qeic init code from platforms to a common function
> >   irqchip/qeic: merge qeic_of_init into qe_ic_init
> >   irqchip/qeic: remove PPCisms for QEIC
> >
> >  MAINTAINERS|   6 +
> >  arch/powerpc/platforms/83xx/km83xx.c   |   1 -
> >  arch/powerpc/platforms/83xx/misc.c |  16 -
> >  arch/powerpc/platforms/83xx/mpc832x_mds.c  |   1 -
> >  arch/powerpc/platforms/83xx/mpc832x_rdb.c  |   1 -
> >  arch/powerpc/platforms/83xx/mpc836x_mds.c  |   1 -
> >  arch/powerpc/platforms/83xx/mpc836x_rdk.c  |   1 -
> >  arch/powerpc/platforms/85xx/corenet_generic.c  |  10 -
> >  arch/powerpc/platforms/85xx/mpc85xx_mds.c  |  15 -
> >  arch/powerpc/platforms/85xx/mpc85xx_rdb.c  |  17 -
> >  arch/powerpc/platforms/85xx/twr_p102x.c|  15 -
> >  drivers/irqchip/Makefile   |   1 +
> >  drivers/{soc/fsl/qe/qe_ic.c => irqchip/irq-qeic.c} | 358 
> > -
> >  drivers/soc/fsl/qe/Makefile|   2 +-
> >  drivers/soc/fsl/qe/qe_ic.h | 103 --
> >  include/soc/fsl/qe/qe_ic.h | 139 
> >  16 files changed, 218 insertions(+), 469 deletions(-)  rename
> > drivers/{soc/fsl/qe/qe_ic.c => irqchip/irq-qeic.c} (58%)  delete mode
> > 100644 drivers/soc/fsl/qe/qe_ic.h  delete mode 100644
> > include/soc/fsl/qe/qe_ic.h
> >
> > --
> > 2.1.0.27.g96db324

[PATCH v2 2/2] devicetree: i2c-hid: Add reset property

[Lin Huang]

Document a "reset" and "assert-reset-us", it can be used for
driver control reset property. And reuse post-power-on-delay-ms
for deassert reset delay.

Signed-off-by: Lin Huang 
---
 Documentation/devicetree/bindings/input/hid-over-i2c.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt 
b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
index 28e8bd8..6ab0eed 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
@@ -31,7 +31,9 @@ device-specific compatible properties, which should be used 
in addition to the
 
 - vdd-supply: phandle of the regulator that provides the supply voltage.
 - post-power-on-delay-ms: time required by the device after enabling its 
regulators
-  before it is ready for communication. Must be used with 'vdd-supply'.
+  or deassert reset pin before it is ready for communication.
+- reset: phandle of the gpio that provides for hid reset pin.
+- assert-reset-us: the device require reset assert time.
 
 Example:
 
-- 
2.7.4

[PATCH v2 2/2] devicetree: i2c-hid: Add reset property

[Lin Huang]

Document a "reset" and "assert-reset-us", it can be used for
driver control reset property. And reuse post-power-on-delay-ms
for deassert reset delay.

Signed-off-by: Lin Huang 
---
 Documentation/devicetree/bindings/input/hid-over-i2c.txt | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/input/hid-over-i2c.txt 
b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
index 28e8bd8..6ab0eed 100644
--- a/Documentation/devicetree/bindings/input/hid-over-i2c.txt
+++ b/Documentation/devicetree/bindings/input/hid-over-i2c.txt
@@ -31,7 +31,9 @@ device-specific compatible properties, which should be used 
in addition to the
 
 - vdd-supply: phandle of the regulator that provides the supply voltage.
 - post-power-on-delay-ms: time required by the device after enabling its 
regulators
-  before it is ready for communication. Must be used with 'vdd-supply'.
+  or deassert reset pin before it is ready for communication.
+- reset: phandle of the gpio that provides for hid reset pin.
+- assert-reset-us: the device require reset assert time.
 
 Example:
 
-- 
2.7.4

[PATCH v2 1/2] HID: i2c-hid: add reset gpio property

[Lin Huang]

some i2c hid devices have reset gpio, need to control
it in the driver.

Signed-off-by: Lin Huang 
---
Changes in v2:
- Add 10us in usleep_range() upper range
- reuse post_power_delay_ms as deassert reset delay
- delete deassert_reset_us property

 drivers/hid/i2c-hid/i2c-hid.c | 61 +++
 include/linux/platform_data/i2c-hid.h |  3 ++
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 9145c21..9f06135 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -793,6 +794,35 @@ struct hid_ll_driver i2c_hid_ll_driver = {
 };
 EXPORT_SYMBOL_GPL(i2c_hid_ll_driver);
 
+static int i2c_hid_hw_power_on(struct i2c_hid *ihid)
+{
+   int ret;
+
+   ret = regulator_enable(ihid->pdata.supply);
+   if (ret < 0)
+   return ret;
+
+   gpiod_set_value_cansleep(ihid->pdata.reset_gpio, 1);
+   if (ihid->pdata.assert_reset_us)
+   usleep_range(ihid->pdata.assert_reset_us,
+ihid->pdata.assert_reset_us + 10);
+
+   gpiod_set_value_cansleep(ihid->pdata.reset_gpio, 0);
+
+   /* use for power supply delay or reset deassert delay */
+   if (ihid->pdata.post_power_delay_ms)
+   msleep(ihid->pdata.post_power_delay_ms);
+
+   return ret;
+}
+
+static int i2c_hid_hw_power_off(struct i2c_hid *ihid)
+{
+   gpiod_set_value_cansleep(ihid->pdata.reset_gpio, 1);
+
+   return regulator_disable(ihid->pdata.supply);
+}
+
 static int i2c_hid_init_irq(struct i2c_client *client)
 {
struct i2c_hid *ihid = i2c_get_clientdata(client);
@@ -934,6 +964,9 @@ static int i2c_hid_of_probe(struct i2c_client *client,
if (!ret)
pdata->post_power_delay_ms = val;
 
+   of_property_read_u32(dev->of_node, "assert-reset-us",
+>assert_reset_us);
+
return 0;
 }
 
@@ -1002,14 +1035,16 @@ static int i2c_hid_probe(struct i2c_client *client,
goto err;
}
 
-   ret = regulator_enable(ihid->pdata.supply);
+   ihid->pdata.reset_gpio = devm_gpiod_get_optional(>dev, "reset",
+GPIOD_OUT_HIGH);
+   if (IS_ERR(ihid->pdata.reset_gpio))
+   goto err;
+
+   ret = i2c_hid_hw_power_on(ihid);
if (ret < 0) {
-   dev_err(>dev, "Failed to enable regulator: %d\n",
-   ret);
+   dev_err(>dev, "Failed to power on: %d\n", ret);
goto err;
}
-   if (ihid->pdata.post_power_delay_ms)
-   msleep(ihid->pdata.post_power_delay_ms);
 
i2c_set_clientdata(client, ihid);
 
@@ -1086,7 +1121,7 @@ static int i2c_hid_probe(struct i2c_client *client,
pm_runtime_disable(>dev);
 
 err_regulator:
-   regulator_disable(ihid->pdata.supply);
+   i2c_hid_hw_power_off(ihid);
 
 err:
i2c_hid_free_buffers(ihid);
@@ -1112,8 +1147,7 @@ static int i2c_hid_remove(struct i2c_client *client)
if (ihid->bufsize)
i2c_hid_free_buffers(ihid);
 
-   regulator_disable(ihid->pdata.supply);
-
+   i2c_hid_hw_power_off(ihid);
kfree(ihid);
 
return 0;
@@ -1165,9 +1199,10 @@ static int i2c_hid_suspend(struct device *dev)
hid_warn(hid, "Failed to enable irq wake: %d\n",
wake_status);
} else {
-   ret = regulator_disable(ihid->pdata.supply);
+   ret = i2c_hid_hw_power_off(ihid);
if (ret < 0)
-   hid_warn(hid, "Failed to disable supply: %d\n", ret);
+   hid_warn(hid, "Failed to disable hw power: %d\n",
+ret);
}
 
return 0;
@@ -1182,11 +1217,9 @@ static int i2c_hid_resume(struct device *dev)
int wake_status;
 
if (!device_may_wakeup(>dev)) {
-   ret = regulator_enable(ihid->pdata.supply);
+   ret = i2c_hid_hw_power_on(ihid);
if (ret < 0)
-   hid_warn(hid, "Failed to enable supply: %d\n", ret);
-   if (ihid->pdata.post_power_delay_ms)
-   msleep(ihid->pdata.post_power_delay_ms);
+   hid_warn(hid, "Failed to enable hw power: %d\n", ret);
} else if (ihid->irq_wake_enabled) {
wake_status = disable_irq_wake(client->irq);
if (!wake_status)
diff --git a/include/linux/platform_data/i2c-hid.h 
b/include/linux/platform_data/i2c-hid.h
index 1fb0882..40f1840 100644
--- a/include/linux/platform_data/i2c-hid.h
+++ b/include/linux/platform_data/i2c-hid.h
@@ -14,6 +14,7 @@
 
 #include 
 
+struct gpio_desc;
 struct regulator;
 
 /**
@@ -37,6 +38,8 @@ struct i2c_hid_platform_data {
u16 

[PATCH v2 1/2] HID: i2c-hid: add reset gpio property

[Lin Huang]

some i2c hid devices have reset gpio, need to control
it in the driver.

Signed-off-by: Lin Huang 
---
Changes in v2:
- Add 10us in usleep_range() upper range
- reuse post_power_delay_ms as deassert reset delay
- delete deassert_reset_us property

 drivers/hid/i2c-hid/i2c-hid.c | 61 +++
 include/linux/platform_data/i2c-hid.h |  3 ++
 2 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 9145c21..9f06135 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -38,6 +38,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include 
@@ -793,6 +794,35 @@ struct hid_ll_driver i2c_hid_ll_driver = {
 };
 EXPORT_SYMBOL_GPL(i2c_hid_ll_driver);
 
+static int i2c_hid_hw_power_on(struct i2c_hid *ihid)
+{
+   int ret;
+
+   ret = regulator_enable(ihid->pdata.supply);
+   if (ret < 0)
+   return ret;
+
+   gpiod_set_value_cansleep(ihid->pdata.reset_gpio, 1);
+   if (ihid->pdata.assert_reset_us)
+   usleep_range(ihid->pdata.assert_reset_us,
+ihid->pdata.assert_reset_us + 10);
+
+   gpiod_set_value_cansleep(ihid->pdata.reset_gpio, 0);
+
+   /* use for power supply delay or reset deassert delay */
+   if (ihid->pdata.post_power_delay_ms)
+   msleep(ihid->pdata.post_power_delay_ms);
+
+   return ret;
+}
+
+static int i2c_hid_hw_power_off(struct i2c_hid *ihid)
+{
+   gpiod_set_value_cansleep(ihid->pdata.reset_gpio, 1);
+
+   return regulator_disable(ihid->pdata.supply);
+}
+
 static int i2c_hid_init_irq(struct i2c_client *client)
 {
struct i2c_hid *ihid = i2c_get_clientdata(client);
@@ -934,6 +964,9 @@ static int i2c_hid_of_probe(struct i2c_client *client,
if (!ret)
pdata->post_power_delay_ms = val;
 
+   of_property_read_u32(dev->of_node, "assert-reset-us",
+>assert_reset_us);
+
return 0;
 }
 
@@ -1002,14 +1035,16 @@ static int i2c_hid_probe(struct i2c_client *client,
goto err;
}
 
-   ret = regulator_enable(ihid->pdata.supply);
+   ihid->pdata.reset_gpio = devm_gpiod_get_optional(>dev, "reset",
+GPIOD_OUT_HIGH);
+   if (IS_ERR(ihid->pdata.reset_gpio))
+   goto err;
+
+   ret = i2c_hid_hw_power_on(ihid);
if (ret < 0) {
-   dev_err(>dev, "Failed to enable regulator: %d\n",
-   ret);
+   dev_err(>dev, "Failed to power on: %d\n", ret);
goto err;
}
-   if (ihid->pdata.post_power_delay_ms)
-   msleep(ihid->pdata.post_power_delay_ms);
 
i2c_set_clientdata(client, ihid);
 
@@ -1086,7 +1121,7 @@ static int i2c_hid_probe(struct i2c_client *client,
pm_runtime_disable(>dev);
 
 err_regulator:
-   regulator_disable(ihid->pdata.supply);
+   i2c_hid_hw_power_off(ihid);
 
 err:
i2c_hid_free_buffers(ihid);
@@ -1112,8 +1147,7 @@ static int i2c_hid_remove(struct i2c_client *client)
if (ihid->bufsize)
i2c_hid_free_buffers(ihid);
 
-   regulator_disable(ihid->pdata.supply);
-
+   i2c_hid_hw_power_off(ihid);
kfree(ihid);
 
return 0;
@@ -1165,9 +1199,10 @@ static int i2c_hid_suspend(struct device *dev)
hid_warn(hid, "Failed to enable irq wake: %d\n",
wake_status);
} else {
-   ret = regulator_disable(ihid->pdata.supply);
+   ret = i2c_hid_hw_power_off(ihid);
if (ret < 0)
-   hid_warn(hid, "Failed to disable supply: %d\n", ret);
+   hid_warn(hid, "Failed to disable hw power: %d\n",
+ret);
}
 
return 0;
@@ -1182,11 +1217,9 @@ static int i2c_hid_resume(struct device *dev)
int wake_status;
 
if (!device_may_wakeup(>dev)) {
-   ret = regulator_enable(ihid->pdata.supply);
+   ret = i2c_hid_hw_power_on(ihid);
if (ret < 0)
-   hid_warn(hid, "Failed to enable supply: %d\n", ret);
-   if (ihid->pdata.post_power_delay_ms)
-   msleep(ihid->pdata.post_power_delay_ms);
+   hid_warn(hid, "Failed to enable hw power: %d\n", ret);
} else if (ihid->irq_wake_enabled) {
wake_status = disable_irq_wake(client->irq);
if (!wake_status)
diff --git a/include/linux/platform_data/i2c-hid.h 
b/include/linux/platform_data/i2c-hid.h
index 1fb0882..40f1840 100644
--- a/include/linux/platform_data/i2c-hid.h
+++ b/include/linux/platform_data/i2c-hid.h
@@ -14,6 +14,7 @@
 
 #include 
 
+struct gpio_desc;
 struct regulator;
 
 /**
@@ -37,6 +38,8 @@ struct i2c_hid_platform_data {
u16 hid_descriptor_address;

Re: [PATCH v2] f2fs: collect prefree segments to avoild write checkpoint fail

[Chao Yu]

On 2017/10/31 10:05, Yunlong Song wrote:
> So I use CHECK_FS config to control it. When CHECK_FS is off, all the 
> other f2fs_bug_on also
> only printk WARNING info rather than trigger BUG_ON.

If this runing out-of-free-segments issue explicitly happens, IMO, its better
to face and fix it.

BTW, it's better to add bug_on here to detect the issue?

Thanks,

> 
> On 2017/10/31 9:59, Chao Yu wrote:
>> On 2017/10/31 9:33, Yunlong Song wrote:
>>> ping...
>>>
>>> On 2017/9/1 20:00, Yunlong Song wrote:
 In come corner case, the reserved segments are used to do gc, and there are
 not enough free segments for write checkpoint to finish its job, then the
 gc process will fail to change the prefree segments to free segments.
>> I agreed to use this in production for robustness, but for upstream, it's 
>> better
>> to investigate and fix this issue rather than covering up it.
>>
>> Thanks,
>>
 Signed-off-by: Yunlong Song 
 ---
fs/f2fs/gc.c | 10 ++
1 file changed, 10 insertions(+)

 diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
 index cd147e7..6552b04 100644
 --- a/fs/f2fs/gc.c
 +++ b/fs/f2fs/gc.c
 @@ -1056,6 +1056,16 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
if (!sync) {
if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
segno = NULL_SEGNO;
 +#ifndef CONFIG_F2FS_CHECK_FS
 +  if (prefree_segments(sbi) &&
 +  has_not_enough_free_secs(sbi,
 +  reserved_sections(sbi), 0)) {
 +  ret = write_checkpoint(sbi, );
 +  if (ret)
 +  goto stop;
 +  sec_freed = 0;
 +  }
 +#endif
goto gc_more;
}

>>
>> .
>>
> 

Re: [PATCH v2] f2fs: collect prefree segments to avoild write checkpoint fail

[Chao Yu]

On 2017/10/31 10:05, Yunlong Song wrote:
> So I use CHECK_FS config to control it. When CHECK_FS is off, all the 
> other f2fs_bug_on also
> only printk WARNING info rather than trigger BUG_ON.

If this runing out-of-free-segments issue explicitly happens, IMO, its better
to face and fix it.

BTW, it's better to add bug_on here to detect the issue?

Thanks,

> 
> On 2017/10/31 9:59, Chao Yu wrote:
>> On 2017/10/31 9:33, Yunlong Song wrote:
>>> ping...
>>>
>>> On 2017/9/1 20:00, Yunlong Song wrote:
 In come corner case, the reserved segments are used to do gc, and there are
 not enough free segments for write checkpoint to finish its job, then the
 gc process will fail to change the prefree segments to free segments.
>> I agreed to use this in production for robustness, but for upstream, it's 
>> better
>> to investigate and fix this issue rather than covering up it.
>>
>> Thanks,
>>
 Signed-off-by: Yunlong Song 
 ---
fs/f2fs/gc.c | 10 ++
1 file changed, 10 insertions(+)

 diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
 index cd147e7..6552b04 100644
 --- a/fs/f2fs/gc.c
 +++ b/fs/f2fs/gc.c
 @@ -1056,6 +1056,16 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
if (!sync) {
if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
segno = NULL_SEGNO;
 +#ifndef CONFIG_F2FS_CHECK_FS
 +  if (prefree_segments(sbi) &&
 +  has_not_enough_free_secs(sbi,
 +  reserved_sections(sbi), 0)) {
 +  ret = write_checkpoint(sbi, );
 +  if (ret)
 +  goto stop;
 +  sec_freed = 0;
 +  }
 +#endif
goto gc_more;
}

>>
>> .
>>
> 

Re: [lkp-robot] [printk] 7f7c60e066: BUG:KASAN:slab-out-of-bounds

[Kees Cook]

On Mon, Oct 30, 2017 at 7:39 PM, Tobin C. Harding  wrote:
> On Mon, Oct 30, 2017 at 05:14:59PM -0700, Kees Cook wrote:
>> On Wed, Oct 25, 2017 at 9:22 AM, kernel test robot
>>  wrote:
>
> thanks for looking at this, I was at a loss as to what (if any) action I
> needed to take.

I have finally learned after several instances that if 0day produces a
report, it's real. :) Even the crazy stuff I haven't been able to
reproduce on my own has come back to bite me later, so now I'm very
attentive to 0day report. ;)

>> > FYI, we noticed the following commit (built with gcc-4.9):
>> >
>> > commit: 7f7c60e0663645e757e520245606fde9c6e326bb ("printk: hash addresses 
>> > printed with %p")
>> > url: 
>> > https://github.com/0day-ci/linux/commits/Tobin-C-Harding/printk-hash-addresses-printed-with-p/20171024-231922
>>
>> It's not clear to me which of the various versions this test ran
>> against, but it seems like the printf self-tests got very confused by
>> the results:
>>
>> > [   40.275423] test_printf: kvasprintf(..., "%p %p", ...) returned 
>> > '3cf9adbe eff717bf', expected '01234567 fedcba98'
>> > [   40.296739] test_printf: vsnprintf(buf, 256, "|%-*p|%*p|", ...) 
>> > returned 19, expected 39
>> > [   40.322776] test_printf: vsnprintf(buf, 16, "|%-*p|%*p|", ...) returned 
>> > 19, expected 39
>> > [   40.334834] test_printf: vsnprintf(buf, 0, "|%-*p|%*p|", ...) returned 
>> > 19, expected 39
>>
>> I assume v10 will fix the width issues, but probably not the value tests...
>
> Oh, so I need to update lib/test_printf.c to cover hashed %p.

Yeah, looks like it.

>> And it claims a use-after-free, too:
>>
>> > [   39.757461] The buggy address belongs to the object at 22cb34bb
>> > [   39.757461]  which belongs to the cache kmalloc-32 of size 32
>> > [   39.757461] The buggy address is located 0 bytes inside of
>> > [   39.757461]  32-byte region [22cb34bb, 24ac3a60)
>>
>> Which becomes rather unreadable, since the address got hashed. :P
>
> So I think we need to patch mm/kasan/report to use %pK instead of %p.

Yeah, looks like it.

> I don't know what I should be doing about
>
> [   39.757461] BUG: KASAN: slab-out-of-bounds in __test+0xee/0x13f

Yeah, I have no idea where this error came from. It does seem
triggered by the printf self-test code, though, so if a KASAN bulid
doesn't show it, maybe a full SLAB DEBUG build will?

-Kees

-- 
Kees Cook
Pixel Security

Re: [lkp-robot] [printk] 7f7c60e066: BUG:KASAN:slab-out-of-bounds

[Kees Cook]

On Mon, Oct 30, 2017 at 7:39 PM, Tobin C. Harding  wrote:
> On Mon, Oct 30, 2017 at 05:14:59PM -0700, Kees Cook wrote:
>> On Wed, Oct 25, 2017 at 9:22 AM, kernel test robot
>>  wrote:
>
> thanks for looking at this, I was at a loss as to what (if any) action I
> needed to take.

I have finally learned after several instances that if 0day produces a
report, it's real. :) Even the crazy stuff I haven't been able to
reproduce on my own has come back to bite me later, so now I'm very
attentive to 0day report. ;)

>> > FYI, we noticed the following commit (built with gcc-4.9):
>> >
>> > commit: 7f7c60e0663645e757e520245606fde9c6e326bb ("printk: hash addresses 
>> > printed with %p")
>> > url: 
>> > https://github.com/0day-ci/linux/commits/Tobin-C-Harding/printk-hash-addresses-printed-with-p/20171024-231922
>>
>> It's not clear to me which of the various versions this test ran
>> against, but it seems like the printf self-tests got very confused by
>> the results:
>>
>> > [   40.275423] test_printf: kvasprintf(..., "%p %p", ...) returned 
>> > '3cf9adbe eff717bf', expected '01234567 fedcba98'
>> > [   40.296739] test_printf: vsnprintf(buf, 256, "|%-*p|%*p|", ...) 
>> > returned 19, expected 39
>> > [   40.322776] test_printf: vsnprintf(buf, 16, "|%-*p|%*p|", ...) returned 
>> > 19, expected 39
>> > [   40.334834] test_printf: vsnprintf(buf, 0, "|%-*p|%*p|", ...) returned 
>> > 19, expected 39
>>
>> I assume v10 will fix the width issues, but probably not the value tests...
>
> Oh, so I need to update lib/test_printf.c to cover hashed %p.

Yeah, looks like it.

>> And it claims a use-after-free, too:
>>
>> > [   39.757461] The buggy address belongs to the object at 22cb34bb
>> > [   39.757461]  which belongs to the cache kmalloc-32 of size 32
>> > [   39.757461] The buggy address is located 0 bytes inside of
>> > [   39.757461]  32-byte region [22cb34bb, 24ac3a60)
>>
>> Which becomes rather unreadable, since the address got hashed. :P
>
> So I think we need to patch mm/kasan/report to use %pK instead of %p.

Yeah, looks like it.

> I don't know what I should be doing about
>
> [   39.757461] BUG: KASAN: slab-out-of-bounds in __test+0xee/0x13f

Yeah, I have no idea where this error came from. It does seem
triggered by the printf self-test code, though, so if a KASAN bulid
doesn't show it, maybe a full SLAB DEBUG build will?

-Kees

-- 
Kees Cook
Pixel Security

Re: [lkp-robot] [printk] 7f7c60e066: BUG:KASAN:slab-out-of-bounds

[Tobin C. Harding]

On Mon, Oct 30, 2017 at 05:14:59PM -0700, Kees Cook wrote:
> On Wed, Oct 25, 2017 at 9:22 AM, kernel test robot
>  wrote:

thanks for looking at this, I was at a loss as to what (if any) action I
needed to take.

> > FYI, we noticed the following commit (built with gcc-4.9):
> >
> > commit: 7f7c60e0663645e757e520245606fde9c6e326bb ("printk: hash addresses 
> > printed with %p")
> > url: 
> > https://github.com/0day-ci/linux/commits/Tobin-C-Harding/printk-hash-addresses-printed-with-p/20171024-231922
> 
> It's not clear to me which of the various versions this test ran
> against, but it seems like the printf self-tests got very confused by
> the results:
> 
> > [   40.275423] test_printf: kvasprintf(..., "%p %p", ...) returned 
> > '3cf9adbe eff717bf', expected '01234567 fedcba98'
> > [   40.296739] test_printf: vsnprintf(buf, 256, "|%-*p|%*p|", ...) returned 
> > 19, expected 39
> > [   40.322776] test_printf: vsnprintf(buf, 16, "|%-*p|%*p|", ...) returned 
> > 19, expected 39
> > [   40.334834] test_printf: vsnprintf(buf, 0, "|%-*p|%*p|", ...) returned 
> > 19, expected 39
> 
> I assume v10 will fix the width issues, but probably not the value tests...

Oh, so I need to update lib/test_printf.c to cover hashed %p.

> And it claims a use-after-free, too:
> 
> > [   39.757461] The buggy address belongs to the object at 22cb34bb
> > [   39.757461]  which belongs to the cache kmalloc-32 of size 32
> > [   39.757461] The buggy address is located 0 bytes inside of
> > [   39.757461]  32-byte region [22cb34bb, 24ac3a60)
> 
> Which becomes rather unreadable, since the address got hashed. :P

So I think we need to patch mm/kasan/report to use %pK instead of %p.

I don't know what I should be doing about

[   39.757461] BUG: KASAN: slab-out-of-bounds in __test+0xee/0x13f

Awesome, thanks,
Tobin.

Re: [lkp-robot] [printk] 7f7c60e066: BUG:KASAN:slab-out-of-bounds

[Tobin C. Harding]

On Mon, Oct 30, 2017 at 05:14:59PM -0700, Kees Cook wrote:
> On Wed, Oct 25, 2017 at 9:22 AM, kernel test robot
>  wrote:

thanks for looking at this, I was at a loss as to what (if any) action I
needed to take.

> > FYI, we noticed the following commit (built with gcc-4.9):
> >
> > commit: 7f7c60e0663645e757e520245606fde9c6e326bb ("printk: hash addresses 
> > printed with %p")
> > url: 
> > https://github.com/0day-ci/linux/commits/Tobin-C-Harding/printk-hash-addresses-printed-with-p/20171024-231922
> 
> It's not clear to me which of the various versions this test ran
> against, but it seems like the printf self-tests got very confused by
> the results:
> 
> > [   40.275423] test_printf: kvasprintf(..., "%p %p", ...) returned 
> > '3cf9adbe eff717bf', expected '01234567 fedcba98'
> > [   40.296739] test_printf: vsnprintf(buf, 256, "|%-*p|%*p|", ...) returned 
> > 19, expected 39
> > [   40.322776] test_printf: vsnprintf(buf, 16, "|%-*p|%*p|", ...) returned 
> > 19, expected 39
> > [   40.334834] test_printf: vsnprintf(buf, 0, "|%-*p|%*p|", ...) returned 
> > 19, expected 39
> 
> I assume v10 will fix the width issues, but probably not the value tests...

Oh, so I need to update lib/test_printf.c to cover hashed %p.

> And it claims a use-after-free, too:
> 
> > [   39.757461] The buggy address belongs to the object at 22cb34bb
> > [   39.757461]  which belongs to the cache kmalloc-32 of size 32
> > [   39.757461] The buggy address is located 0 bytes inside of
> > [   39.757461]  32-byte region [22cb34bb, 24ac3a60)
> 
> Which becomes rather unreadable, since the address got hashed. :P

So I think we need to patch mm/kasan/report to use %pK instead of %p.

I don't know what I should be doing about

[   39.757461] BUG: KASAN: slab-out-of-bounds in __test+0xee/0x13f

Awesome, thanks,
Tobin.

RE: linux-next: manual merge of the spi-nor tree with the imx-mxs tree

[Z.q. Hou]

Hi Cyrille,

> -Original Message-
> From: Cyrille Pitchen [mailto:cyrille.pitc...@wedev4u.fr]
> Sent: 2017年10月31日 8:43
> To: Mark Brown ; Yuan Yao ; Z.q.
> Hou ; Rob Herring ; Shawn Guo
> ; Philipp Puschmann 
> Cc: linux-arm-ker...@lists.infradead.org; Linux-Next Mailing List
> ; Linux Kernel Mailing List
> ; Marek Vasut 
> Subject: Re: linux-next: manual merge of the spi-nor tree with the imx-mxs 
> tree
> 
> Hi all,
> 
> + Marek
> 
> Mark, thanks for this report.
> 
> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add
> sst25wf040b and en25s64 to sip-nor list" was not submitted to the linux-mtd
> mailing list hence was neither reviewed nor acked by any spi-nor maintainer. 
> If
> so, such a patch should then be taken from the spi-nor/next branch of the
> l2-mtd tree.
> 
> So Shawn, could you please remove this patch from your tree ?
> 
> Yuan, could you please submit your patch to the linux-mtd mailing list for
> proper review ?

Only this single patch is needed to submit to linux-mtd mailing list, right?

Thanks,
Zhiqiang

> Best regards,
> 
> Cyrille
> 
> 
> Le 30/10/2017 à 19:15, Mark Brown a écrit :
> > Hi Cyrille,
> >
> > Today's linux-next merge of the spi-nor tree got a conflict in:
> >
> >   Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >
> > between commit:
> >
> >   b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to
> > sip-nor list")
> >
> > from the imx-mxs tree and commit:
> >
> >282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
> >
> > from the spi-nor tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This is
> > now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your
> > tree is submitted for merging.  You may also want to consider
> > cooperating with the maintainer of the conflicting tree to minimise
> > any particularly complex conflicts.
> >
> > diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > index 4cab5d85cf6f,956bb046e599..
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > @@@ -13,7 -13,7 +13,8 @@@ Required properties
> >at25df321a
> >at25df641
> >at26df081a
> >  + en25s64
> > +  mr25h128
> >mr25h256
> >mr25h10
> >mr25h40
> >

RE: linux-next: manual merge of the spi-nor tree with the imx-mxs tree

[Z.q. Hou]

Hi Cyrille,

> -Original Message-
> From: Cyrille Pitchen [mailto:cyrille.pitc...@wedev4u.fr]
> Sent: 2017年10月31日 8:43
> To: Mark Brown ; Yuan Yao ; Z.q.
> Hou ; Rob Herring ; Shawn Guo
> ; Philipp Puschmann 
> Cc: linux-arm-ker...@lists.infradead.org; Linux-Next Mailing List
> ; Linux Kernel Mailing List
> ; Marek Vasut 
> Subject: Re: linux-next: manual merge of the spi-nor tree with the imx-mxs 
> tree
> 
> Hi all,
> 
> + Marek
> 
> Mark, thanks for this report.
> 
> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add
> sst25wf040b and en25s64 to sip-nor list" was not submitted to the linux-mtd
> mailing list hence was neither reviewed nor acked by any spi-nor maintainer. 
> If
> so, such a patch should then be taken from the spi-nor/next branch of the
> l2-mtd tree.
> 
> So Shawn, could you please remove this patch from your tree ?
> 
> Yuan, could you please submit your patch to the linux-mtd mailing list for
> proper review ?

Only this single patch is needed to submit to linux-mtd mailing list, right?

Thanks,
Zhiqiang

> Best regards,
> 
> Cyrille
> 
> 
> Le 30/10/2017 à 19:15, Mark Brown a écrit :
> > Hi Cyrille,
> >
> > Today's linux-next merge of the spi-nor tree got a conflict in:
> >
> >   Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> >
> > between commit:
> >
> >   b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to
> > sip-nor list")
> >
> > from the imx-mxs tree and commit:
> >
> >282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
> >
> > from the spi-nor tree.
> >
> > I fixed it up (see below) and can carry the fix as necessary. This is
> > now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your
> > tree is submitted for merging.  You may also want to consider
> > cooperating with the maintainer of the conflicting tree to minimise
> > any particularly complex conflicts.
> >
> > diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > index 4cab5d85cf6f,956bb046e599..
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > @@@ -13,7 -13,7 +13,8 @@@ Required properties
> >at25df321a
> >at25df641
> >at26df081a
> >  + en25s64
> > +  mr25h128
> >mr25h256
> >mr25h10
> >mr25h40
> >

Re: [PATCH] net: caif: chnl_net: remove unnecessary null check in chnl_recv_cb

[Gustavo A. R. Silva]

Quoting "Gustavo A. R. Silva" :


Hi,

Quoting "Gustavo A. R. Silva" :


container_of is never null, so this null check is unnecessary.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
net/caif/chnl_net.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 922ac1d..489298d 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -77,8 +77,6 @@ static int chnl_recv_cb(struct cflayer *layr,  
struct cfpkt *pkt)

u8 buf;

priv = container_of(layr, struct chnl_net, chnl);
-   if (!priv)
-   return -EINVAL;

skb = (struct sk_buff *) cfpkt_tonative(pkt);

--
2.7.4


Please, ignore this patch.

I just realized that function chnl_recv_cb is being called only  
during initialization:


chnl_init_module() -> rtnl_link_register() -> ipcaif_net_setup() ->  
chnl_recv_cb():




Well, here ipcaif_net_setup stores a pointer to chnl_recv_cb in a  
structure. It doesn't call it.



static void ipcaif_net_setup(struct net_device *dev)
{
[...]
priv = netdev_priv(dev);
priv->chnl.receive = chnl_recv_cb;

[...]
}

static struct rtnl_link_ops ipcaif_link_ops __read_mostly = {
.kind   = "caif",
.priv_size  = sizeof(struct chnl_net),
.setup  = ipcaif_net_setup,
.maxtype= IFLA_CAIF_MAX,
.policy = ipcaif_policy,
.newlink= ipcaif_newlink,
.changelink = ipcaif_changelink,
.get_size   = ipcaif_get_size,
.fill_info  = ipcaif_fill_info,

};

static int __init chnl_init_module(void)
{
return rtnl_link_register(_link_ops);
}



--
Gustavo A. R. Silva

Re: [PATCH] net: caif: chnl_net: remove unnecessary null check in chnl_recv_cb

[Gustavo A. R. Silva]

Quoting "Gustavo A. R. Silva" :


Hi,

Quoting "Gustavo A. R. Silva" :


container_of is never null, so this null check is unnecessary.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva 
---
net/caif/chnl_net.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/net/caif/chnl_net.c b/net/caif/chnl_net.c
index 922ac1d..489298d 100644
--- a/net/caif/chnl_net.c
+++ b/net/caif/chnl_net.c
@@ -77,8 +77,6 @@ static int chnl_recv_cb(struct cflayer *layr,  
struct cfpkt *pkt)

u8 buf;

priv = container_of(layr, struct chnl_net, chnl);
-   if (!priv)
-   return -EINVAL;

skb = (struct sk_buff *) cfpkt_tonative(pkt);

--
2.7.4


Please, ignore this patch.

I just realized that function chnl_recv_cb is being called only  
during initialization:


chnl_init_module() -> rtnl_link_register() -> ipcaif_net_setup() ->  
chnl_recv_cb():




Well, here ipcaif_net_setup stores a pointer to chnl_recv_cb in a  
structure. It doesn't call it.



static void ipcaif_net_setup(struct net_device *dev)
{
[...]
priv = netdev_priv(dev);
priv->chnl.receive = chnl_recv_cb;

[...]
}

static struct rtnl_link_ops ipcaif_link_ops __read_mostly = {
.kind   = "caif",
.priv_size  = sizeof(struct chnl_net),
.setup  = ipcaif_net_setup,
.maxtype= IFLA_CAIF_MAX,
.policy = ipcaif_policy,
.newlink= ipcaif_newlink,
.changelink = ipcaif_changelink,
.get_size   = ipcaif_get_size,
.fill_info  = ipcaif_fill_info,

};

static int __init chnl_init_module(void)
{
return rtnl_link_register(_link_ops);
}



--
Gustavo A. R. Silva

[PATCH] genirq: add mutex and rcu locking to irq_desc_tree

[Masahiro Yamada]

Add a mutex to prevent concurrency on the updater side of the
irq_desc radix tree.

Add rcu_read_lock/unlock to the reader side so that lifetimes of
leaf pointers of the radix tree are correctly managed.

Signed-off-by: Masahiro Yamada 
---

 kernel/irq/irqdesc.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 82afb7e..928c896 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -312,21 +313,32 @@ static void irq_sysfs_add(int irq, struct irq_desc *desc) 
{}
 #endif /* CONFIG_SYSFS */
 
 static RADIX_TREE(irq_desc_tree, GFP_KERNEL);
+static DEFINE_MUTEX(irq_desc_tree_mutex);
 
 static void irq_insert_desc(unsigned int irq, struct irq_desc *desc)
 {
+   mutex_lock(_desc_tree_mutex);
radix_tree_insert(_desc_tree, irq, desc);
+   mutex_unlock(_desc_tree_mutex);
 }
 
 struct irq_desc *irq_to_desc(unsigned int irq)
 {
-   return radix_tree_lookup(_desc_tree, irq);
+   struct irq_desc *desc;
+
+   rcu_read_lock();
+   desc = radix_tree_lookup(_desc_tree, irq);
+   rcu_read_unlock();
+
+   return desc;
 }
 EXPORT_SYMBOL(irq_to_desc);
 
 static void delete_irq_desc(unsigned int irq)
 {
+   mutex_lock(_desc_tree_mutex);
radix_tree_delete(_desc_tree, irq);
+   mutex_unlock(_desc_tree_mutex);
 }
 
 #ifdef CONFIG_SMP
-- 
2.7.4

[PATCH] genirq: add mutex and rcu locking to irq_desc_tree

[Masahiro Yamada]

Add a mutex to prevent concurrency on the updater side of the
irq_desc radix tree.

Add rcu_read_lock/unlock to the reader side so that lifetimes of
leaf pointers of the radix tree are correctly managed.

Signed-off-by: Masahiro Yamada 
---

 kernel/irq/irqdesc.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/kernel/irq/irqdesc.c b/kernel/irq/irqdesc.c
index 82afb7e..928c896 100644
--- a/kernel/irq/irqdesc.c
+++ b/kernel/irq/irqdesc.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -312,21 +313,32 @@ static void irq_sysfs_add(int irq, struct irq_desc *desc) 
{}
 #endif /* CONFIG_SYSFS */
 
 static RADIX_TREE(irq_desc_tree, GFP_KERNEL);
+static DEFINE_MUTEX(irq_desc_tree_mutex);
 
 static void irq_insert_desc(unsigned int irq, struct irq_desc *desc)
 {
+   mutex_lock(_desc_tree_mutex);
radix_tree_insert(_desc_tree, irq, desc);
+   mutex_unlock(_desc_tree_mutex);
 }
 
 struct irq_desc *irq_to_desc(unsigned int irq)
 {
-   return radix_tree_lookup(_desc_tree, irq);
+   struct irq_desc *desc;
+
+   rcu_read_lock();
+   desc = radix_tree_lookup(_desc_tree, irq);
+   rcu_read_unlock();
+
+   return desc;
 }
 EXPORT_SYMBOL(irq_to_desc);
 
 static void delete_irq_desc(unsigned int irq)
 {
+   mutex_lock(_desc_tree_mutex);
radix_tree_delete(_desc_tree, irq);
+   mutex_unlock(_desc_tree_mutex);
 }
 
 #ifdef CONFIG_SMP
-- 
2.7.4

Re: [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add Goldfish PIC driver

[Marc Zyngier]

On Mon, Oct 30 2017 at 12:56:33 pm GMT, Aleksandar Markovic 
 wrote:
> From: Miodrag Dinic 
>
> Add device driver for a virtual programmable interrupt controller
>
> The virtual PIC is designed as a device tree-based interrupt controller.
>
> The compatible string used by OS for binding the driver is
> "google,goldfish-pic".
>
> Signed-off-by: Miodrag Dinic 
> Signed-off-by: Goran Ferenc 
> Signed-off-by: Aleksandar Markovic 

[...]

> diff --git a/drivers/irqchip/irq-goldfish-pic.c 
> b/drivers/irqchip/irq-goldfish-pic.c
> new file mode 100644
> index 000..48fb773
> --- /dev/null
> +++ b/drivers/irqchip/irq-goldfish-pic.c

[...]

> +static int __init goldfish_pic_of_init(struct device_node *of_node,
> +struct device_node *parent)
> +{
> + struct goldfish_pic_data *gfpic;
> + struct irq_chip_generic *gc;
> + struct irq_chip_type *ct;
> + unsigned int parent_irq;
> + int ret = 0;
> +
> + gfpic = kzalloc(sizeof(*gfpic), GFP_KERNEL);
> + if (!gfpic) {
> + ret = -ENOMEM;
> + goto out_err;
> + }
> +
> + parent_irq = irq_of_parse_and_map(of_node, 0);
> + if (!parent_irq) {
> + pr_err("Failed to map Goldfish PIC parent IRQ\n");
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + gfpic->base = of_iomap(of_node, 0);
> + if (!gfpic->base) {
> + pr_err("Failed to map Goldfish PIC base\n");
> + ret = -ENOMEM;
> + goto out_unmap_irq;
> + }
> +
> + /* Mask interrupts. */
> + writel(1, gfpic->base + GFPIC_REG_IRQ_DISABLE_ALL);
> +
> + gc = irq_alloc_generic_chip("GFPIC", 1, GFPIC_IRQ_BASE, gfpic->base,
> + handle_level_irq);
> +
> + ct = gc->chip_types;

And what if the allocation fails?

> + ct->regs.enable = GFPIC_REG_IRQ_ENABLE;
> + ct->regs.disable = GFPIC_REG_IRQ_DISABLE;
> + ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
> + ct->chip.irq_mask = irq_gc_mask_disable_reg;
> +
> + irq_setup_generic_chip(gc, IRQ_MSK(GFPIC_NR_IRQS), 0, 0,
> +IRQ_NOPROBE | IRQ_LEVEL);
> +
> + gfpic->irq_domain = irq_domain_add_legacy(of_node, GFPIC_NR_IRQS,
> +   GFPIC_IRQ_BASE, 0,
> +   _irq_domain_ops,
> +   NULL);
> + if (!gfpic->irq_domain) {
> + pr_err("Failed to add irqdomain for Goldfish PIC\n");
> + ret = -EINVAL;
> + goto out_iounmap;
> + }
> +
> + irq_set_chained_handler_and_data(parent_irq,
> +  goldfish_pic_cascade, gfpic);
> +
> + pr_info("Successfully registered Goldfish PIC\n");
> + return 0;
> +
> +out_iounmap:
> + iounmap(gfpic->base);
> +out_unmap_irq:
> + irq_dispose_mapping(parent_irq); 
> +out_free:
> + kfree(gfpic);
> +out_err:
> + return ret;
> +}
> +
> +IRQCHIP_DECLARE(google_gf_pic, "google,goldfish-pic", goldfish_pic_of_init);

Thanks,

M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH v6 2/5] irqchip/irq-goldfish-pic: Add Goldfish PIC driver

[Marc Zyngier]

On Mon, Oct 30 2017 at 12:56:33 pm GMT, Aleksandar Markovic 
 wrote:
> From: Miodrag Dinic 
>
> Add device driver for a virtual programmable interrupt controller
>
> The virtual PIC is designed as a device tree-based interrupt controller.
>
> The compatible string used by OS for binding the driver is
> "google,goldfish-pic".
>
> Signed-off-by: Miodrag Dinic 
> Signed-off-by: Goran Ferenc 
> Signed-off-by: Aleksandar Markovic 

[...]

> diff --git a/drivers/irqchip/irq-goldfish-pic.c 
> b/drivers/irqchip/irq-goldfish-pic.c
> new file mode 100644
> index 000..48fb773
> --- /dev/null
> +++ b/drivers/irqchip/irq-goldfish-pic.c

[...]

> +static int __init goldfish_pic_of_init(struct device_node *of_node,
> +struct device_node *parent)
> +{
> + struct goldfish_pic_data *gfpic;
> + struct irq_chip_generic *gc;
> + struct irq_chip_type *ct;
> + unsigned int parent_irq;
> + int ret = 0;
> +
> + gfpic = kzalloc(sizeof(*gfpic), GFP_KERNEL);
> + if (!gfpic) {
> + ret = -ENOMEM;
> + goto out_err;
> + }
> +
> + parent_irq = irq_of_parse_and_map(of_node, 0);
> + if (!parent_irq) {
> + pr_err("Failed to map Goldfish PIC parent IRQ\n");
> + ret = -EINVAL;
> + goto out_free;
> + }
> +
> + gfpic->base = of_iomap(of_node, 0);
> + if (!gfpic->base) {
> + pr_err("Failed to map Goldfish PIC base\n");
> + ret = -ENOMEM;
> + goto out_unmap_irq;
> + }
> +
> + /* Mask interrupts. */
> + writel(1, gfpic->base + GFPIC_REG_IRQ_DISABLE_ALL);
> +
> + gc = irq_alloc_generic_chip("GFPIC", 1, GFPIC_IRQ_BASE, gfpic->base,
> + handle_level_irq);
> +
> + ct = gc->chip_types;

And what if the allocation fails?

> + ct->regs.enable = GFPIC_REG_IRQ_ENABLE;
> + ct->regs.disable = GFPIC_REG_IRQ_DISABLE;
> + ct->chip.irq_unmask = irq_gc_unmask_enable_reg;
> + ct->chip.irq_mask = irq_gc_mask_disable_reg;
> +
> + irq_setup_generic_chip(gc, IRQ_MSK(GFPIC_NR_IRQS), 0, 0,
> +IRQ_NOPROBE | IRQ_LEVEL);
> +
> + gfpic->irq_domain = irq_domain_add_legacy(of_node, GFPIC_NR_IRQS,
> +   GFPIC_IRQ_BASE, 0,
> +   _irq_domain_ops,
> +   NULL);
> + if (!gfpic->irq_domain) {
> + pr_err("Failed to add irqdomain for Goldfish PIC\n");
> + ret = -EINVAL;
> + goto out_iounmap;
> + }
> +
> + irq_set_chained_handler_and_data(parent_irq,
> +  goldfish_pic_cascade, gfpic);
> +
> + pr_info("Successfully registered Goldfish PIC\n");
> + return 0;
> +
> +out_iounmap:
> + iounmap(gfpic->base);
> +out_unmap_irq:
> + irq_dispose_mapping(parent_irq); 
> +out_free:
> + kfree(gfpic);
> +out_err:
> + return ret;
> +}
> +
> +IRQCHIP_DECLARE(google_gf_pic, "google,goldfish-pic", goldfish_pic_of_init);

Thanks,

M.
-- 
Jazz is not dead. It just smells funny.

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Minchan Kim]

On Mon, Oct 30, 2017 at 09:02:30AM +0100, Michal Hocko wrote:
> On Mon 30-10-17 08:57:13, Minchan Kim wrote:
> [...]
> > Although it's better than old, we can make it simple, still.
> > 
> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > index 291c4b534658..f50d5a48f03a 100644
> > --- a/include/linux/swapops.h
> > +++ b/include/linux/swapops.h
> > @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
> > return (entry.val >> SWP_TYPE_SHIFT(entry));
> >  }
> >  
> > +extern struct swap_info_struct *swap_info[];
> > +
> > +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
> > +{
> > +   return swap_info[swp_type(entry)];
> > +}
> > +
> >  /*
> >   * Extract the `offset' field from a swp_entry_t.  The swp_entry_t is in
> >   * arch-independent format
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 378262d3a197..a0fe2d54ad09 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -554,6 +554,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
> > gfp_mask,
> > struct vm_area_struct *vma, unsigned long addr)
> >  {
> > struct page *page;
> > +   struct swap_info_struct *si = swp_si(entry);
> 
> Aren't you accessing beyond the array here?

I couldn't understand what you intend. Could you explain what case does it 
accesses
beyond the arrary?

Thanks.

> 
> > unsigned long entry_offset = swp_offset(entry);
> > unsigned long offset = entry_offset;
> > unsigned long start_offset, end_offset;
> > @@ -572,6 +573,9 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
> > gfp_mask,
> > if (!start_offset)  /* First page is swap header. */
> > start_offset++;
> >  
> > +   if (end_offset >= si->max)
> > +   end_offset = si->max - 1;
> > +
> > blk_start_plug();
> > for (offset = start_offset; offset <= end_offset ; offset++) {
> > /* Ok, do the async read-ahead now */
> 
> -- 
> Michal Hocko
> SUSE Labs
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree

[Shawn Guo]

On Tue, Oct 31, 2017 at 01:42:30AM +0100, Cyrille Pitchen wrote:
> Hi all,
> 
> + Marek
> 
> Mark, thanks for this report.
> 
> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add 
> sst25wf040b
> and en25s64 to sip-nor list" was not submitted to the linux-mtd mailing list
> hence was neither reviewed nor acked by any spi-nor maintainer. If so, such a
> patch should then be taken from the spi-nor/next branch of the l2-mtd tree.

I see the patch was Acked by DT maintainer and so applied it as part of
the series.  I will leave such mtd bindings patch to l2-mtd maintainers
in the future.

> So Shawn, could you please remove this patch from your tree ?

I have already sent the patch to my upstream maintainers (arm-soc).  I
guess it's okay to leave such trivial conflict to Linus.  @Arnd, what do
you think?

Shawn

> Yuan, could you please submit your patch to the linux-mtd mailing list for
> proper review ?
> 
> Best regards,
> 
> Cyrille
> 
> 
> Le 30/10/2017 à 19:15, Mark Brown a écrit :
> > Hi Cyrille,
> > 
> > Today's linux-next merge of the spi-nor tree got a conflict in:
> > 
> >   Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > 
> > between commit:
> > 
> >   b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to sip-nor 
> > list")
> > 
> > from the imx-mxs tree and commit:
> > 
> >282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
> > 
> > from the spi-nor tree.
> > 
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging.  You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> > 
> > diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > index 4cab5d85cf6f,956bb046e599..
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > @@@ -13,7 -13,7 +13,8 @@@ Required properties
> >at25df321a
> >at25df641
> >at26df081a
> >  + en25s64
> > +  mr25h128
> >mr25h256
> >mr25h10
> >mr25h40
> > 
> 

Re: [PATCH -mm -V2] mm, swap: Fix false error message in __swp_swapcount()

[Minchan Kim]

On Mon, Oct 30, 2017 at 09:02:30AM +0100, Michal Hocko wrote:
> On Mon 30-10-17 08:57:13, Minchan Kim wrote:
> [...]
> > Although it's better than old, we can make it simple, still.
> > 
> > diff --git a/include/linux/swapops.h b/include/linux/swapops.h
> > index 291c4b534658..f50d5a48f03a 100644
> > --- a/include/linux/swapops.h
> > +++ b/include/linux/swapops.h
> > @@ -41,6 +41,13 @@ static inline unsigned swp_type(swp_entry_t entry)
> > return (entry.val >> SWP_TYPE_SHIFT(entry));
> >  }
> >  
> > +extern struct swap_info_struct *swap_info[];
> > +
> > +static inline struct swap_info_struct *swp_si(swp_entry_t entry)
> > +{
> > +   return swap_info[swp_type(entry)];
> > +}
> > +
> >  /*
> >   * Extract the `offset' field from a swp_entry_t.  The swp_entry_t is in
> >   * arch-independent format
> > diff --git a/mm/swap_state.c b/mm/swap_state.c
> > index 378262d3a197..a0fe2d54ad09 100644
> > --- a/mm/swap_state.c
> > +++ b/mm/swap_state.c
> > @@ -554,6 +554,7 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
> > gfp_mask,
> > struct vm_area_struct *vma, unsigned long addr)
> >  {
> > struct page *page;
> > +   struct swap_info_struct *si = swp_si(entry);
> 
> Aren't you accessing beyond the array here?

I couldn't understand what you intend. Could you explain what case does it 
accesses
beyond the arrary?

Thanks.

> 
> > unsigned long entry_offset = swp_offset(entry);
> > unsigned long offset = entry_offset;
> > unsigned long start_offset, end_offset;
> > @@ -572,6 +573,9 @@ struct page *swapin_readahead(swp_entry_t entry, gfp_t 
> > gfp_mask,
> > if (!start_offset)  /* First page is swap header. */
> > start_offset++;
> >  
> > +   if (end_offset >= si->max)
> > +   end_offset = si->max - 1;
> > +
> > blk_start_plug();
> > for (offset = start_offset; offset <= end_offset ; offset++) {
> > /* Ok, do the async read-ahead now */
> 
> -- 
> Michal Hocko
> SUSE Labs
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org;> em...@kvack.org 

Re: linux-next: manual merge of the spi-nor tree with the imx-mxs tree

[Shawn Guo]

On Tue, Oct 31, 2017 at 01:42:30AM +0100, Cyrille Pitchen wrote:
> Hi all,
> 
> + Marek
> 
> Mark, thanks for this report.
> 
> Shawn, Yuan, if I don't make a mistake, patch "dt-bindings: mtd: add 
> sst25wf040b
> and en25s64 to sip-nor list" was not submitted to the linux-mtd mailing list
> hence was neither reviewed nor acked by any spi-nor maintainer. If so, such a
> patch should then be taken from the spi-nor/next branch of the l2-mtd tree.

I see the patch was Acked by DT maintainer and so applied it as part of
the series.  I will leave such mtd bindings patch to l2-mtd maintainers
in the future.

> So Shawn, could you please remove this patch from your tree ?

I have already sent the patch to my upstream maintainers (arm-soc).  I
guess it's okay to leave such trivial conflict to Linus.  @Arnd, what do
you think?

Shawn

> Yuan, could you please submit your patch to the linux-mtd mailing list for
> proper review ?
> 
> Best regards,
> 
> Cyrille
> 
> 
> Le 30/10/2017 à 19:15, Mark Brown a écrit :
> > Hi Cyrille,
> > 
> > Today's linux-next merge of the spi-nor tree got a conflict in:
> > 
> >   Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > 
> > between commit:
> > 
> >   b07815d4eaf65 ("dt-bindings: mtd: add sst25wf040b and en25s64 to sip-nor 
> > list")
> > 
> > from the imx-mxs tree and commit:
> > 
> >282e45dc64d1 ("mtd: spi-nor: Add support for mr25h128")
> > 
> > from the spi-nor tree.
> > 
> > I fixed it up (see below) and can carry the fix as necessary. This
> > is now fixed as far as linux-next is concerned, but any non trivial
> > conflicts should be mentioned to your upstream maintainer when your tree
> > is submitted for merging.  You may also want to consider cooperating
> > with the maintainer of the conflicting tree to minimise any particularly
> > complex conflicts.
> > 
> > diff --cc Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > index 4cab5d85cf6f,956bb046e599..
> > --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt
> > @@@ -13,7 -13,7 +13,8 @@@ Required properties
> >at25df321a
> >at25df641
> >at26df081a
> >  + en25s64
> > +  mr25h128
> >mr25h256
> >mr25h10
> >mr25h40
> > 
> 

[PATCH v2] iio: adc: aspeed: Deassert reset in probe

[Joel Stanley]

The ASPEED SoC must deassert a reset in order to use the ADC peripheral.

The device tree bindings are updated to document the resets phandle, and
the example is updated to match what is expected for both the reset and
clock phandle. Note that the bindings should have always had the reset
controller, as the hardware is unusable without it.

Signed-off-by: Joel Stanley 
---
v2:
 - Ensure disabling path unwinds in opposite order as the enable path
 - Note that the bindings were incorrect without the reset phandle,
 and for the system to be usable we must update them. No one was
 (successfully) using these bindings/driver before without out of tree
 hacks in mach-aspeed, as it would not have worked.

 .../devicetree/bindings/iio/adc/aspeed_adc.txt |  4 +++-
 drivers/iio/adc/aspeed_adc.c   | 25 --
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt 
b/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
index 674e133b7cd7..034fc2ba100e 100644
--- a/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
@@ -8,6 +8,7 @@ Required properties:
 - reg: memory window mapping address and length
 - clocks: Input clock used to derive the sample clock. Expected to be the
   SoC's APB clock.
+- resets: Reset controller phandle
 - #io-channel-cells: Must be set to <1> to indicate channels are selected
  by index.
 
@@ -15,6 +16,7 @@ Example:
adc@1e6e9000 {
compatible = "aspeed,ast2400-adc";
reg = <0x1e6e9000 0xb0>;
-   clocks = <_apb>;
+   clocks = < ASPEED_CLK_APB>;
+   resets = < ASPEED_RESET_ADC>;
#io-channel-cells = <1>;
};
diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 8a958d5f1905..327a49ba1991 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -53,11 +54,12 @@ struct aspeed_adc_model_data {
 };
 
 struct aspeed_adc_data {
-   struct device   *dev;
-   void __iomem*base;
-   spinlock_t  clk_lock;
-   struct clk_hw   *clk_prescaler;
-   struct clk_hw   *clk_scaler;
+   struct device   *dev;
+   void __iomem*base;
+   spinlock_t  clk_lock;
+   struct clk_hw   *clk_prescaler;
+   struct clk_hw   *clk_scaler;
+   struct reset_control*rst;
 };
 
 #define ASPEED_CHAN(_idx, _data_reg_addr) {\
@@ -217,6 +219,15 @@ static int aspeed_adc_probe(struct platform_device *pdev)
goto scaler_error;
}
 
+   data->rst = devm_reset_control_get_exclusive(>dev, NULL);
+   if (IS_ERR(data->rst)) {
+   dev_err(>dev,
+   "invalid or missing reset controller device tree 
entry");
+   ret = PTR_ERR(data->rst);
+   goto reset_error;
+   }
+   reset_control_deassert(data->rst);
+
model_data = of_device_get_match_data(>dev);
 
if (model_data->wait_init_sequence) {
@@ -263,9 +274,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
writel(ASPEED_OPERATION_MODE_POWER_DOWN,
data->base + ASPEED_REG_ENGINE_CONTROL);
clk_disable_unprepare(data->clk_scaler->clk);
+reset_error:
+   reset_control_assert(data->rst);
 clk_enable_error:
clk_hw_unregister_divider(data->clk_scaler);
-
 scaler_error:
clk_hw_unregister_divider(data->clk_prescaler);
return ret;
@@ -280,6 +292,7 @@ static int aspeed_adc_remove(struct platform_device *pdev)
writel(ASPEED_OPERATION_MODE_POWER_DOWN,
data->base + ASPEED_REG_ENGINE_CONTROL);
clk_disable_unprepare(data->clk_scaler->clk);
+   reset_control_assert(data->rst);
clk_hw_unregister_divider(data->clk_scaler);
clk_hw_unregister_divider(data->clk_prescaler);
 
-- 
2.14.1

[PATCH v2] iio: adc: aspeed: Deassert reset in probe

[Joel Stanley]

The ASPEED SoC must deassert a reset in order to use the ADC peripheral.

The device tree bindings are updated to document the resets phandle, and
the example is updated to match what is expected for both the reset and
clock phandle. Note that the bindings should have always had the reset
controller, as the hardware is unusable without it.

Signed-off-by: Joel Stanley 
---
v2:
 - Ensure disabling path unwinds in opposite order as the enable path
 - Note that the bindings were incorrect without the reset phandle,
 and for the system to be usable we must update them. No one was
 (successfully) using these bindings/driver before without out of tree
 hacks in mach-aspeed, as it would not have worked.

 .../devicetree/bindings/iio/adc/aspeed_adc.txt |  4 +++-
 drivers/iio/adc/aspeed_adc.c   | 25 --
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt 
b/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
index 674e133b7cd7..034fc2ba100e 100644
--- a/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
+++ b/Documentation/devicetree/bindings/iio/adc/aspeed_adc.txt
@@ -8,6 +8,7 @@ Required properties:
 - reg: memory window mapping address and length
 - clocks: Input clock used to derive the sample clock. Expected to be the
   SoC's APB clock.
+- resets: Reset controller phandle
 - #io-channel-cells: Must be set to <1> to indicate channels are selected
  by index.
 
@@ -15,6 +16,7 @@ Example:
adc@1e6e9000 {
compatible = "aspeed,ast2400-adc";
reg = <0x1e6e9000 0xb0>;
-   clocks = <_apb>;
+   clocks = < ASPEED_CLK_APB>;
+   resets = < ASPEED_RESET_ADC>;
#io-channel-cells = <1>;
};
diff --git a/drivers/iio/adc/aspeed_adc.c b/drivers/iio/adc/aspeed_adc.c
index 8a958d5f1905..327a49ba1991 100644
--- a/drivers/iio/adc/aspeed_adc.c
+++ b/drivers/iio/adc/aspeed_adc.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -53,11 +54,12 @@ struct aspeed_adc_model_data {
 };
 
 struct aspeed_adc_data {
-   struct device   *dev;
-   void __iomem*base;
-   spinlock_t  clk_lock;
-   struct clk_hw   *clk_prescaler;
-   struct clk_hw   *clk_scaler;
+   struct device   *dev;
+   void __iomem*base;
+   spinlock_t  clk_lock;
+   struct clk_hw   *clk_prescaler;
+   struct clk_hw   *clk_scaler;
+   struct reset_control*rst;
 };
 
 #define ASPEED_CHAN(_idx, _data_reg_addr) {\
@@ -217,6 +219,15 @@ static int aspeed_adc_probe(struct platform_device *pdev)
goto scaler_error;
}
 
+   data->rst = devm_reset_control_get_exclusive(>dev, NULL);
+   if (IS_ERR(data->rst)) {
+   dev_err(>dev,
+   "invalid or missing reset controller device tree 
entry");
+   ret = PTR_ERR(data->rst);
+   goto reset_error;
+   }
+   reset_control_deassert(data->rst);
+
model_data = of_device_get_match_data(>dev);
 
if (model_data->wait_init_sequence) {
@@ -263,9 +274,10 @@ static int aspeed_adc_probe(struct platform_device *pdev)
writel(ASPEED_OPERATION_MODE_POWER_DOWN,
data->base + ASPEED_REG_ENGINE_CONTROL);
clk_disable_unprepare(data->clk_scaler->clk);
+reset_error:
+   reset_control_assert(data->rst);
 clk_enable_error:
clk_hw_unregister_divider(data->clk_scaler);
-
 scaler_error:
clk_hw_unregister_divider(data->clk_prescaler);
return ret;
@@ -280,6 +292,7 @@ static int aspeed_adc_remove(struct platform_device *pdev)
writel(ASPEED_OPERATION_MODE_POWER_DOWN,
data->base + ASPEED_REG_ENGINE_CONTROL);
clk_disable_unprepare(data->clk_scaler->clk);
+   reset_control_assert(data->rst);
clk_hw_unregister_divider(data->clk_scaler);
clk_hw_unregister_divider(data->clk_prescaler);
 
-- 
2.14.1

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Joe Perches]

On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > Here is the behaviour that this set implements.
> > > 
> > > For kpt_restrict==0
> > > 
> > > Randomness not ready:
> > >   printed with %p:  (pointer)  # NOTE: with padding
> > > Valid pointer:
> > >   printed with %pK: deadbeefdeadbeef
> > >   printed with %p:  0xdeadbeef
> > >   malformed specifier (eg %i):  0xdeadbeef
> > 
> > I really think we can't include SPECIAL unless _every_ callsite of %p
> > is actually doing "0x%p", and then we're replacing all of those. We're
> > not doing that, though...
> > 
> > $ git grep '%p\b' | wc -l
> > 12766
> > $ git grep '0x%p\b' | wc -l
> > 18370x
> > 
> > If we need some kind of special marking that this is a hashed
> > variable, that should be something other than "0x". If we're using the
> > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > should be used instead? Then the (rare) callers with 0x become
> > "0x(hash:)" and naked callers produce "(hash:)".
> > 
> > I think the first step for this is to just leave SPECIAL out.
> 
> Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> address with the first 32 bits masked to zero. The intent being to _not_
> change the output format from what it currently is. So it will look like
> this; 
> 
>   c09e81d0
> 
> What do you think?
> 
> Amusingly I think this whole conversation is going to come up again
> when we do %pa, in inverse, since %pa currently does us SPECIAL.

I once sent a patch set to remove SPECIAL from %pa
and add 0x where necessary.

https://patchwork.kernel.org/patch/3875471/

After that didn't happen, I removed the duplicated
0x%pa with a sed.

https://patchwork.kernel.org/patch/8509421/

Sending a treewide sed patch would be fine with me.

Re: [PATCH V8 0/2] printk: hash addresses printed with %p

[Joe Perches]

On Tue, 2017-10-31 at 09:33 +1100, Tobin C. Harding wrote:
> On Mon, Oct 30, 2017 at 03:03:21PM -0700, Kees Cook wrote:
> > On Wed, Oct 25, 2017 at 7:53 PM, Tobin C. Harding  wrote:
> > > Here is the behaviour that this set implements.
> > > 
> > > For kpt_restrict==0
> > > 
> > > Randomness not ready:
> > >   printed with %p:  (pointer)  # NOTE: with padding
> > > Valid pointer:
> > >   printed with %pK: deadbeefdeadbeef
> > >   printed with %p:  0xdeadbeef
> > >   malformed specifier (eg %i):  0xdeadbeef
> > 
> > I really think we can't include SPECIAL unless _every_ callsite of %p
> > is actually doing "0x%p", and then we're replacing all of those. We're
> > not doing that, though...
> > 
> > $ git grep '%p\b' | wc -l
> > 12766
> > $ git grep '0x%p\b' | wc -l
> > 18370x
> > 
> > If we need some kind of special marking that this is a hashed
> > variable, that should be something other than "0x". If we're using the
> > existing "(null)" and new "(pointer)" text, maybe "(hash:xx)"
> > should be used instead? Then the (rare) callers with 0x become
> > "0x(hash:)" and naked callers produce "(hash:)".
> > 
> > I think the first step for this is to just leave SPECIAL out.
> 
> Thanks Kees. V9 leaves SPECIAL out. Also V9 prints the whole 64 bit
> address with the first 32 bits masked to zero. The intent being to _not_
> change the output format from what it currently is. So it will look like
> this; 
> 
>   c09e81d0
> 
> What do you think?
> 
> Amusingly I think this whole conversation is going to come up again
> when we do %pa, in inverse, since %pa currently does us SPECIAL.

I once sent a patch set to remove SPECIAL from %pa
and add 0x where necessary.

https://patchwork.kernel.org/patch/3875471/

After that didn't happen, I removed the duplicated
0x%pa with a sed.

https://patchwork.kernel.org/patch/8509421/

Sending a treewide sed patch would be fine with me.

Re: Kernel crash in free_pipe_info()

[Linus Torvalds]

On Mon, Oct 30, 2017 at 6:19 PM, Cong Wang  wrote:
>
> 1. The faulty addresses are all near 0001, with one exception
> of null (which is the most recent one)

Well, they're at 8(%rax), except for that last case.

And in every case (_including_ that last case), %rax has a very
interesting pattern.. That's the (bad) buf->ops pointer that  was
loaded from the somehow corrupted "buf".

The values in all cases are

fffa
fffd
fff1
fff7
fff4
fffa
fffd
fffd
fffa
ffe8
fff1
fff7

which kind of looks like a 32-bit error value. So we have (n, val, (errno)):

  1 -24 (EMFILE)
  2 -15 (ENOTBLK)
  1 -12 (ENOMEM)
  2 -9 (EBADF)
  3 -6 (ENXIO)
  3 -3 (ESRCH)

none of which makes any sense to me, but it's an interesting pattern
nonetheless.

> 2. R12 register, which should map to the local vairable 'i', is always 0x8
> at the time of crash.

So _if_ this is some kind of use-after-free thing, and the allocation
got re-used for something else, that might just be related to whatever
ends up being the offset that is filled in with the (int) error
number.

Except the offset is that %r12*0x28+0x10, so we're talking a byte
offset of 330 bytes into the allocation, and apparently the eight
previous (0-7) iterations were fine.

Which is really odd.

I'm not seeing anything that makes sense. I'll have to think about this.

I'm assuming you don't have slub debugging enabled, and no way to
enable it and try to catch this?

   Linus

Re: Kernel crash in free_pipe_info()

[Linus Torvalds]

On Mon, Oct 30, 2017 at 6:19 PM, Cong Wang  wrote:
>
> 1. The faulty addresses are all near 0001, with one exception
> of null (which is the most recent one)

Well, they're at 8(%rax), except for that last case.

And in every case (_including_ that last case), %rax has a very
interesting pattern.. That's the (bad) buf->ops pointer that  was
loaded from the somehow corrupted "buf".

The values in all cases are

fffa
fffd
fff1
fff7
fff4
fffa
fffd
fffd
fffa
ffe8
fff1
fff7

which kind of looks like a 32-bit error value. So we have (n, val, (errno)):

  1 -24 (EMFILE)
  2 -15 (ENOTBLK)
  1 -12 (ENOMEM)
  2 -9 (EBADF)
  3 -6 (ENXIO)
  3 -3 (ESRCH)

none of which makes any sense to me, but it's an interesting pattern
nonetheless.

> 2. R12 register, which should map to the local vairable 'i', is always 0x8
> at the time of crash.

So _if_ this is some kind of use-after-free thing, and the allocation
got re-used for something else, that might just be related to whatever
ends up being the offset that is filled in with the (int) error
number.

Except the offset is that %r12*0x28+0x10, so we're talking a byte
offset of 330 bytes into the allocation, and apparently the eight
previous (0-7) iterations were fine.

Which is really odd.

I'm not seeing anything that makes sense. I'll have to think about this.

I'm assuming you don't have slub debugging enabled, and no way to
enable it and try to catch this?

   Linus

Re: [PATCH v2] f2fs: collect prefree segments to avoild write checkpoint fail

[Yunlong Song]

So I use CHECK_FS config to control it. When CHECK_FS is off, all the 
other f2fs_bug_on also

only printk WARNING info rather than trigger BUG_ON.

On 2017/10/31 9:59, Chao Yu wrote:

On 2017/10/31 9:33, Yunlong Song wrote:

ping...

On 2017/9/1 20:00, Yunlong Song wrote:

In come corner case, the reserved segments are used to do gc, and there are
not enough free segments for write checkpoint to finish its job, then the
gc process will fail to change the prefree segments to free segments.

I agreed to use this in production for robustness, but for upstream, it's better
to investigate and fix this issue rather than covering up it.

Thanks,


Signed-off-by: Yunlong Song 
---
   fs/f2fs/gc.c | 10 ++
   1 file changed, 10 insertions(+)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index cd147e7..6552b04 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1056,6 +1056,16 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
if (!sync) {
if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
segno = NULL_SEGNO;
+#ifndef CONFIG_F2FS_CHECK_FS
+   if (prefree_segments(sbi) &&
+   has_not_enough_free_secs(sbi,
+   reserved_sections(sbi), 0)) {
+   ret = write_checkpoint(sbi, );
+   if (ret)
+   goto stop;
+   sec_freed = 0;
+   }
+#endif
goto gc_more;
}
   


.



--
Thanks,
Yunlong Song

Re: [PATCH v2] f2fs: collect prefree segments to avoild write checkpoint fail

[Yunlong Song]

So I use CHECK_FS config to control it. When CHECK_FS is off, all the 
other f2fs_bug_on also

only printk WARNING info rather than trigger BUG_ON.

On 2017/10/31 9:59, Chao Yu wrote:

On 2017/10/31 9:33, Yunlong Song wrote:

ping...

On 2017/9/1 20:00, Yunlong Song wrote:

In come corner case, the reserved segments are used to do gc, and there are
not enough free segments for write checkpoint to finish its job, then the
gc process will fail to change the prefree segments to free segments.

I agreed to use this in production for robustness, but for upstream, it's better
to investigate and fix this issue rather than covering up it.

Thanks,


Signed-off-by: Yunlong Song 
---
   fs/f2fs/gc.c | 10 ++
   1 file changed, 10 insertions(+)

diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
index cd147e7..6552b04 100644
--- a/fs/f2fs/gc.c
+++ b/fs/f2fs/gc.c
@@ -1056,6 +1056,16 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
if (!sync) {
if (has_not_enough_free_secs(sbi, sec_freed, 0)) {
segno = NULL_SEGNO;
+#ifndef CONFIG_F2FS_CHECK_FS
+   if (prefree_segments(sbi) &&
+   has_not_enough_free_secs(sbi,
+   reserved_sections(sbi), 0)) {
+   ret = write_checkpoint(sbi, );
+   if (ret)
+   goto stop;
+   sec_freed = 0;
+   }
+#endif
goto gc_more;
}
   


.



--
Thanks,
Yunlong Song