Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore

[Thomas Gleixner]

On Fri, 4 May 2018, Paul E. McKenney wrote:
> On Fri, May 04, 2018 at 11:38:37PM -0500, Eric W. Biederman wrote:
> > > (Me, I would run rcutorture scenario TREE03 for an extended time period
> > > on b4abf91047cf with your patch applied.
> 
> And with lockdep enabled, which TREE03 does not do by default.

Will run that again to make sure.

Thanks,

tglx

Re: [PATCH 2/2] dmaengine: sprd: Add Spreadtrum DMA configuration

[kbuild test robot]

Hi Eric,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on slave-dma/next]
[also build test WARNING on next-20180504]
[cannot apply to linus/master v4.17-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dmaengine-sprd-Optimize-the-sprd_dma_prep_dma_memcpy/20180505-071137
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/dma/sprd-dma.c:780:57: sparse: mixing different enum types
   drivers/dma/sprd-dma.c:780:57: int enum dma_slave_buswidth  versus
   drivers/dma/sprd-dma.c:780:57: int enum sprd_dma_datawidth
   drivers/dma/sprd-dma.c:787:57: sparse: mixing different enum types
   drivers/dma/sprd-dma.c:787:57: int enum dma_slave_buswidth  versus
   drivers/dma/sprd-dma.c:787:57: int enum sprd_dma_datawidth

vim +780 drivers/dma/sprd-dma.c

   755  
   756  static struct dma_async_tx_descriptor *
   757  sprd_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
   758 unsigned int sglen, enum dma_transfer_direction 
dir,
   759 unsigned long flags, void *context)
   760  {
   761  struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
   762  struct sprd_dma_config *slave_cfg = >slave_cfg;
   763  struct sprd_dma_desc *sdesc;
   764  struct scatterlist *sg;
   765  int ret, i;
   766  
   767  /* TODO: now we only support one sg for each DMA configuration. 
*/
   768  if (!is_slave_direction(dir) || sglen > 1)
   769  return NULL;
   770  
   771  sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
   772  if (!sdesc)
   773  return NULL;
   774  
   775  for_each_sg(sgl, sg, sglen, i) {
   776  if (dir == DMA_MEM_TO_DEV) {
   777  slave_cfg->src_addr = sg_dma_address(sg);
   778  slave_cfg->dst_addr = slave_cfg->cfg.dst_addr;
   779  slave_cfg->src_step =
 > 780  
 > sprd_dma_get_step(slave_cfg->cfg.src_addr_width);
   781  slave_cfg->dst_step = SPRD_DMA_NONE_STEP;
   782  } else {
   783  slave_cfg->src_addr = slave_cfg->cfg.src_addr;
   784  slave_cfg->dst_addr = sg_dma_address(sg);
   785  slave_cfg->src_step = SPRD_DMA_NONE_STEP;
   786  slave_cfg->dst_step =
   787  
sprd_dma_get_step(slave_cfg->cfg.dst_addr_width);
   788  }
   789  
   790  slave_cfg->block_len = sg_dma_len(sg);
   791  slave_cfg->transcation_len = sg_dma_len(sg);
   792  }
   793  
   794  slave_cfg->req_mode =
   795  (flags >> SPRD_DMA_REQ_SHIFT) & SPRD_DMA_REQ_MODE_MASK;
   796  slave_cfg->int_mode = flags & SPRD_DMA_INT_MASK;
   797  
   798  ret = sprd_dma_config(chan, sdesc, slave_cfg);
   799  if (ret) {
   800  kfree(sdesc);
   801  return NULL;
   802  }
   803  
   804  return vchan_tx_prep(>vc, >vd, flags);
   805  }
   806  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore

[Thomas Gleixner]

On Fri, 4 May 2018, Paul E. McKenney wrote:
> On Fri, May 04, 2018 at 11:38:37PM -0500, Eric W. Biederman wrote:
> > > (Me, I would run rcutorture scenario TREE03 for an extended time period
> > > on b4abf91047cf with your patch applied.
> 
> And with lockdep enabled, which TREE03 does not do by default.

Will run that again to make sure.

Thanks,

tglx

Re: [PATCH 2/2] dmaengine: sprd: Add Spreadtrum DMA configuration

[kbuild test robot]

Hi Eric,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on slave-dma/next]
[also build test WARNING on next-20180504]
[cannot apply to linus/master v4.17-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dmaengine-sprd-Optimize-the-sprd_dma_prep_dma_memcpy/20180505-071137
base:   https://git.kernel.org/pub/scm/linux/kernel/git/vkoul/slave-dma.git next
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

>> drivers/dma/sprd-dma.c:780:57: sparse: mixing different enum types
   drivers/dma/sprd-dma.c:780:57: int enum dma_slave_buswidth  versus
   drivers/dma/sprd-dma.c:780:57: int enum sprd_dma_datawidth
   drivers/dma/sprd-dma.c:787:57: sparse: mixing different enum types
   drivers/dma/sprd-dma.c:787:57: int enum dma_slave_buswidth  versus
   drivers/dma/sprd-dma.c:787:57: int enum sprd_dma_datawidth

vim +780 drivers/dma/sprd-dma.c

   755  
   756  static struct dma_async_tx_descriptor *
   757  sprd_dma_prep_slave_sg(struct dma_chan *chan, struct scatterlist *sgl,
   758 unsigned int sglen, enum dma_transfer_direction 
dir,
   759 unsigned long flags, void *context)
   760  {
   761  struct sprd_dma_chn *schan = to_sprd_dma_chan(chan);
   762  struct sprd_dma_config *slave_cfg = >slave_cfg;
   763  struct sprd_dma_desc *sdesc;
   764  struct scatterlist *sg;
   765  int ret, i;
   766  
   767  /* TODO: now we only support one sg for each DMA configuration. 
*/
   768  if (!is_slave_direction(dir) || sglen > 1)
   769  return NULL;
   770  
   771  sdesc = kzalloc(sizeof(*sdesc), GFP_NOWAIT);
   772  if (!sdesc)
   773  return NULL;
   774  
   775  for_each_sg(sgl, sg, sglen, i) {
   776  if (dir == DMA_MEM_TO_DEV) {
   777  slave_cfg->src_addr = sg_dma_address(sg);
   778  slave_cfg->dst_addr = slave_cfg->cfg.dst_addr;
   779  slave_cfg->src_step =
 > 780  
 > sprd_dma_get_step(slave_cfg->cfg.src_addr_width);
   781  slave_cfg->dst_step = SPRD_DMA_NONE_STEP;
   782  } else {
   783  slave_cfg->src_addr = slave_cfg->cfg.src_addr;
   784  slave_cfg->dst_addr = sg_dma_address(sg);
   785  slave_cfg->src_step = SPRD_DMA_NONE_STEP;
   786  slave_cfg->dst_step =
   787  
sprd_dma_get_step(slave_cfg->cfg.dst_addr_width);
   788  }
   789  
   790  slave_cfg->block_len = sg_dma_len(sg);
   791  slave_cfg->transcation_len = sg_dma_len(sg);
   792  }
   793  
   794  slave_cfg->req_mode =
   795  (flags >> SPRD_DMA_REQ_SHIFT) & SPRD_DMA_REQ_MODE_MASK;
   796  slave_cfg->int_mode = flags & SPRD_DMA_INT_MASK;
   797  
   798  ret = sprd_dma_config(chan, sdesc, slave_cfg);
   799  if (ret) {
   800  kfree(sdesc);
   801  return NULL;
   802  }
   803  
   804  return vchan_tx_prep(>vc, >vd, flags);
   805  }
   806  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 2/2] Revert "f2fs: add ovp valid_blocks check for bg gc victim to fg_gc"

[Chao Yu]

Ping,

On 2018/4/27 10:55, Chao Yu wrote:
> Hi Jaegeuk,
> 
> Missed this patch, or any problem in it?
> 
> Thanks,
> 
> On 2018/4/26 17:05, Chao Yu wrote:
>> For extreme case:
>> 10 section, op = 10%, no_fggc_threshold = 90%
>> All section usage: 85% 85% 85% 85% 90% 90% 95% 95% 95% 95%
>>
>> During foreground GC, if we skip select dirty section whose usage
>> is larger than no_fggc_threshold, we can only recycle 80% invalid
>> space from four 85% usage sections and two 90% usage sections,
>> result in encountering out-of-space issue.
>>
>> This reverts commit e93b9865251a0503d83fd570e7d5a7c8bc351715 to
>> fix this issue, besides, we keep the logic that we scan all dirty
>> section when searching a victim, so that GC can select victim with
>> least valid blocks.
>>
>> Signed-off-by: Chao Yu 
>> ---
>>  fs/f2fs/f2fs.h|  3 ---
>>  fs/f2fs/gc.c  | 16 
>>  fs/f2fs/segment.h |  9 -
>>  3 files changed, 28 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 64e3677998d8..9f8b327272df 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1192,9 +1192,6 @@ struct f2fs_sb_info {
>>  struct f2fs_gc_kthread  *gc_thread; /* GC thread */
>>  unsigned int cur_victim_sec;/* current victim section num */
>>  
>> -/* threshold for converting bg victims for fg */
>> -u64 fggc_threshold;
>> -
>>  /* threshold for gc trials on pinned files */
>>  u64 gc_pin_file_threshold;
>>  
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 3134dd781252..62eba4a71123 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -263,10 +263,6 @@ static unsigned int check_bg_victims(struct 
>> f2fs_sb_info *sbi)
>>  for_each_set_bit(secno, dirty_i->victim_secmap, MAIN_SECS(sbi)) {
>>  if (sec_usage_check(sbi, secno))
>>  continue;
>> -
>> -if (no_fggc_candidate(sbi, secno))
>> -continue;
>> -
>>  clear_bit(secno, dirty_i->victim_secmap);
>>  return GET_SEG_FROM_SEC(sbi, secno);
>>  }
>> @@ -406,9 +402,6 @@ static int get_victim_by_default(struct f2fs_sb_info 
>> *sbi,
>>  goto next;
>>  if (gc_type == BG_GC && test_bit(secno, dirty_i->victim_secmap))
>>  goto next;
>> -if (gc_type == FG_GC && p.alloc_mode == LFS &&
>> -no_fggc_candidate(sbi, secno))
>> -goto next;
>>  
>>  cost = get_gc_cost(sbi, segno, );
>>  
>> @@ -1152,17 +1145,8 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>>  
>>  void build_gc_manager(struct f2fs_sb_info *sbi)
>>  {
>> -u64 main_count, resv_count, ovp_count;
>> -
>>  DIRTY_I(sbi)->v_ops = _v_ops;
>>  
>> -/* threshold of # of valid blocks in a section for victims of FG_GC */
>> -main_count = SM_I(sbi)->main_segments << sbi->log_blocks_per_seg;
>> -resv_count = SM_I(sbi)->reserved_segments << sbi->log_blocks_per_seg;
>> -ovp_count = SM_I(sbi)->ovp_segments << sbi->log_blocks_per_seg;
>> -
>> -sbi->fggc_threshold = div64_u64((main_count - ovp_count) *
>> -BLKS_PER_SEC(sbi), (main_count - resv_count));
>>  sbi->gc_pin_file_threshold = DEF_GC_FAILED_PINNED_FILES;
>>  
>>  /* give warm/cold data area from slower device */
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 7702b054689c..c385daabcb67 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -774,15 +774,6 @@ static inline block_t sum_blk_addr(struct f2fs_sb_info 
>> *sbi, int base, int type)
>>  - (base + 1) + type;
>>  }
>>  
>> -static inline bool no_fggc_candidate(struct f2fs_sb_info *sbi,
>> -unsigned int secno)
>> -{
>> -if (get_valid_blocks(sbi, GET_SEG_FROM_SEC(sbi, secno), true) >
>> -sbi->fggc_threshold)
>> -return true;
>> -return false;
>> -}
>> -
>>  static inline bool sec_usage_check(struct f2fs_sb_info *sbi, unsigned int 
>> secno)
>>  {
>>  if (IS_CURSEC(sbi, secno) || (sbi->cur_victim_sec == secno))
>>
> 
> 
> .
> 

Re: [PATCH 2/2] Revert "f2fs: add ovp valid_blocks check for bg gc victim to fg_gc"

[Chao Yu]

Ping,

On 2018/4/27 10:55, Chao Yu wrote:
> Hi Jaegeuk,
> 
> Missed this patch, or any problem in it?
> 
> Thanks,
> 
> On 2018/4/26 17:05, Chao Yu wrote:
>> For extreme case:
>> 10 section, op = 10%, no_fggc_threshold = 90%
>> All section usage: 85% 85% 85% 85% 90% 90% 95% 95% 95% 95%
>>
>> During foreground GC, if we skip select dirty section whose usage
>> is larger than no_fggc_threshold, we can only recycle 80% invalid
>> space from four 85% usage sections and two 90% usage sections,
>> result in encountering out-of-space issue.
>>
>> This reverts commit e93b9865251a0503d83fd570e7d5a7c8bc351715 to
>> fix this issue, besides, we keep the logic that we scan all dirty
>> section when searching a victim, so that GC can select victim with
>> least valid blocks.
>>
>> Signed-off-by: Chao Yu 
>> ---
>>  fs/f2fs/f2fs.h|  3 ---
>>  fs/f2fs/gc.c  | 16 
>>  fs/f2fs/segment.h |  9 -
>>  3 files changed, 28 deletions(-)
>>
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 64e3677998d8..9f8b327272df 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1192,9 +1192,6 @@ struct f2fs_sb_info {
>>  struct f2fs_gc_kthread  *gc_thread; /* GC thread */
>>  unsigned int cur_victim_sec;/* current victim section num */
>>  
>> -/* threshold for converting bg victims for fg */
>> -u64 fggc_threshold;
>> -
>>  /* threshold for gc trials on pinned files */
>>  u64 gc_pin_file_threshold;
>>  
>> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
>> index 3134dd781252..62eba4a71123 100644
>> --- a/fs/f2fs/gc.c
>> +++ b/fs/f2fs/gc.c
>> @@ -263,10 +263,6 @@ static unsigned int check_bg_victims(struct 
>> f2fs_sb_info *sbi)
>>  for_each_set_bit(secno, dirty_i->victim_secmap, MAIN_SECS(sbi)) {
>>  if (sec_usage_check(sbi, secno))
>>  continue;
>> -
>> -if (no_fggc_candidate(sbi, secno))
>> -continue;
>> -
>>  clear_bit(secno, dirty_i->victim_secmap);
>>  return GET_SEG_FROM_SEC(sbi, secno);
>>  }
>> @@ -406,9 +402,6 @@ static int get_victim_by_default(struct f2fs_sb_info 
>> *sbi,
>>  goto next;
>>  if (gc_type == BG_GC && test_bit(secno, dirty_i->victim_secmap))
>>  goto next;
>> -if (gc_type == FG_GC && p.alloc_mode == LFS &&
>> -no_fggc_candidate(sbi, secno))
>> -goto next;
>>  
>>  cost = get_gc_cost(sbi, segno, );
>>  
>> @@ -1152,17 +1145,8 @@ int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
>>  
>>  void build_gc_manager(struct f2fs_sb_info *sbi)
>>  {
>> -u64 main_count, resv_count, ovp_count;
>> -
>>  DIRTY_I(sbi)->v_ops = _v_ops;
>>  
>> -/* threshold of # of valid blocks in a section for victims of FG_GC */
>> -main_count = SM_I(sbi)->main_segments << sbi->log_blocks_per_seg;
>> -resv_count = SM_I(sbi)->reserved_segments << sbi->log_blocks_per_seg;
>> -ovp_count = SM_I(sbi)->ovp_segments << sbi->log_blocks_per_seg;
>> -
>> -sbi->fggc_threshold = div64_u64((main_count - ovp_count) *
>> -BLKS_PER_SEC(sbi), (main_count - resv_count));
>>  sbi->gc_pin_file_threshold = DEF_GC_FAILED_PINNED_FILES;
>>  
>>  /* give warm/cold data area from slower device */
>> diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
>> index 7702b054689c..c385daabcb67 100644
>> --- a/fs/f2fs/segment.h
>> +++ b/fs/f2fs/segment.h
>> @@ -774,15 +774,6 @@ static inline block_t sum_blk_addr(struct f2fs_sb_info 
>> *sbi, int base, int type)
>>  - (base + 1) + type;
>>  }
>>  
>> -static inline bool no_fggc_candidate(struct f2fs_sb_info *sbi,
>> -unsigned int secno)
>> -{
>> -if (get_valid_blocks(sbi, GET_SEG_FROM_SEC(sbi, secno), true) >
>> -sbi->fggc_threshold)
>> -return true;
>> -return false;
>> -}
>> -
>>  static inline bool sec_usage_check(struct f2fs_sb_info *sbi, unsigned int 
>> secno)
>>  {
>>  if (IS_CURSEC(sbi, secno) || (sbi->cur_victim_sec == secno))
>>
> 
> 
> .
> 

Re: [PATCH] f2fs: fix to wait IO writeback in __revoke_inmem_pages()

[Chao Yu]

Ping,

On 2018/4/27 10:37, Chao Yu wrote:
> On 2018/4/27 0:36, Jaegeuk Kim wrote:
>> On 04/26, Chao Yu wrote:
>>> On 2018/4/26 23:48, Jaegeuk Kim wrote:
 On 04/26, Chao Yu wrote:
> Thread A  Thread B
> - f2fs_ioc_commit_atomic_write
>  - commit_inmem_pages
>   - f2fs_submit_merged_write_cond
>   : write data
>   - write_checkpoint
>- do_checkpoint
>: commit all node within CP
>-> SPO
>   - f2fs_do_sync_file
>- file_write_and_wait_range
>: wait data writeback
>
> In above race condition, data/node can be flushed in reversed order when
> coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
> atomic written data being corrupted.

 Wait, what is the problem here? Thread B could succeed checkpoint, there is
 no problem. If it fails, there is no fsync mark where we can recover it, so
>>>
>>> Node is flushed by checkpoint before data, with reversed order, that's the 
>>> problem.
>>
>> What do you mean? Data should be in disk, in order to proceed checkpoint.
> 
> 1. thread A: commit_inmem_pages submit data into block layer, but haven't 
> waited
> it writeback.
> 2. thread A: commit_inmem_pages update related node.
> 3. thread B: do checkpoint, flush all nodes to disk
> 4. SPOR
> 
> Then, atomic file becomes corrupted since nodes is flushed before data.
> 
> Thanks,
> 
>>
>>>
>>> Thanks,
>>>
 we can just ignore the last written data as nothing.

>
> This patch adds f2fs_wait_on_page_writeback in __revoke_inmem_pages() to
> keep data and node of atomic file being flushed orderly.
>
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/file.c| 4 
>  fs/f2fs/segment.c | 3 +++
>  2 files changed, 7 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index be7578774a47..a352804af244 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -217,6 +217,9 @@ static int f2fs_do_sync_file(struct file *file, 
> loff_t start, loff_t end,
>  
>   trace_f2fs_sync_file_enter(inode);
>  
> + if (atomic)
> + goto write_done;
> +
>   /* if fdatasync is triggered, let's do in-place-update */
>   if (datasync || get_dirty_pages(inode) <= SM_I(sbi)->min_fsync_blocks)
>   set_inode_flag(inode, FI_NEED_IPU);
> @@ -228,6 +231,7 @@ static int f2fs_do_sync_file(struct file *file, 
> loff_t start, loff_t end,
>   return ret;
>   }
>  
> +write_done:
>   /* if the inode is dirty, let's recover all the time */
>   if (!f2fs_skip_inode_update(inode, datasync)) {
>   f2fs_write_inode(inode, NULL);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 584483426584..9ca3d0a43d93 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -230,6 +230,8 @@ static int __revoke_inmem_pages(struct inode *inode,
>  
>   lock_page(page);
>  
> + f2fs_wait_on_page_writeback(page, DATA, true);
> +
>   if (recover) {
>   struct dnode_of_data dn;
>   struct node_info ni;
> @@ -415,6 +417,7 @@ static int __commit_inmem_pages(struct inode *inode)
>   /* drop all uncommitted pages */
>   __revoke_inmem_pages(inode, >inmem_pages, true, false);
>   } else {
> + /* wait all committed IOs writeback and release them from list 
> */
>   __revoke_inmem_pages(inode, _list, false, false);
>   }
>  
> -- 
> 2.15.0.55.gc2ece9dc4de6
>>
>> .
>>
> 
> 
> .
> 

Re: [PATCH] f2fs: fix to wait IO writeback in __revoke_inmem_pages()

[Chao Yu]

Ping,

On 2018/4/27 10:37, Chao Yu wrote:
> On 2018/4/27 0:36, Jaegeuk Kim wrote:
>> On 04/26, Chao Yu wrote:
>>> On 2018/4/26 23:48, Jaegeuk Kim wrote:
 On 04/26, Chao Yu wrote:
> Thread A  Thread B
> - f2fs_ioc_commit_atomic_write
>  - commit_inmem_pages
>   - f2fs_submit_merged_write_cond
>   : write data
>   - write_checkpoint
>- do_checkpoint
>: commit all node within CP
>-> SPO
>   - f2fs_do_sync_file
>- file_write_and_wait_range
>: wait data writeback
>
> In above race condition, data/node can be flushed in reversed order when
> coming a checkpoint before f2fs_do_sync_file, after SPOR, it results in
> atomic written data being corrupted.

 Wait, what is the problem here? Thread B could succeed checkpoint, there is
 no problem. If it fails, there is no fsync mark where we can recover it, so
>>>
>>> Node is flushed by checkpoint before data, with reversed order, that's the 
>>> problem.
>>
>> What do you mean? Data should be in disk, in order to proceed checkpoint.
> 
> 1. thread A: commit_inmem_pages submit data into block layer, but haven't 
> waited
> it writeback.
> 2. thread A: commit_inmem_pages update related node.
> 3. thread B: do checkpoint, flush all nodes to disk
> 4. SPOR
> 
> Then, atomic file becomes corrupted since nodes is flushed before data.
> 
> Thanks,
> 
>>
>>>
>>> Thanks,
>>>
 we can just ignore the last written data as nothing.

>
> This patch adds f2fs_wait_on_page_writeback in __revoke_inmem_pages() to
> keep data and node of atomic file being flushed orderly.
>
> Signed-off-by: Chao Yu 
> ---
>  fs/f2fs/file.c| 4 
>  fs/f2fs/segment.c | 3 +++
>  2 files changed, 7 insertions(+)
>
> diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
> index be7578774a47..a352804af244 100644
> --- a/fs/f2fs/file.c
> +++ b/fs/f2fs/file.c
> @@ -217,6 +217,9 @@ static int f2fs_do_sync_file(struct file *file, 
> loff_t start, loff_t end,
>  
>   trace_f2fs_sync_file_enter(inode);
>  
> + if (atomic)
> + goto write_done;
> +
>   /* if fdatasync is triggered, let's do in-place-update */
>   if (datasync || get_dirty_pages(inode) <= SM_I(sbi)->min_fsync_blocks)
>   set_inode_flag(inode, FI_NEED_IPU);
> @@ -228,6 +231,7 @@ static int f2fs_do_sync_file(struct file *file, 
> loff_t start, loff_t end,
>   return ret;
>   }
>  
> +write_done:
>   /* if the inode is dirty, let's recover all the time */
>   if (!f2fs_skip_inode_update(inode, datasync)) {
>   f2fs_write_inode(inode, NULL);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 584483426584..9ca3d0a43d93 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -230,6 +230,8 @@ static int __revoke_inmem_pages(struct inode *inode,
>  
>   lock_page(page);
>  
> + f2fs_wait_on_page_writeback(page, DATA, true);
> +
>   if (recover) {
>   struct dnode_of_data dn;
>   struct node_info ni;
> @@ -415,6 +417,7 @@ static int __commit_inmem_pages(struct inode *inode)
>   /* drop all uncommitted pages */
>   __revoke_inmem_pages(inode, >inmem_pages, true, false);
>   } else {
> + /* wait all committed IOs writeback and release them from list 
> */
>   __revoke_inmem_pages(inode, _list, false, false);
>   }
>  
> -- 
> 2.15.0.55.gc2ece9dc4de6
>>
>> .
>>
> 
> 
> .
> 

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

[kbuild test robot]

Hi Xiaotong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc3 next-20180504]
[cannot apply to j.anaszewski-leds/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dt-bindings-leds-Add-SC27xx-breathing-light-controller-documentation/20180504-200830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

smatch warnings:
drivers/leds/leds-sc27xx-bltc.c:337 sc27xx_led_probe() warn: unsigned 'reg' is 
never less than zero.

vim +/reg +337 drivers/leds/leds-sc27xx-bltc.c

   299  
   300  static int sc27xx_led_probe(struct platform_device *pdev)
   301  {
   302  struct device *dev = >dev;
   303  struct device_node *np = dev->of_node, *child;
   304  struct sc27xx_led_priv *priv;
   305  u32 base, count, reg;
   306  int err;
   307  
   308  count = of_get_child_count(np);
   309  if (!count || count > SC27XX_LEDS_MAX)
   310  return -EINVAL;
   311  
   312  err = of_property_read_u32(np, "reg", );
   313  if (err) {
   314  dev_err(dev, "fail to get reg of property\n");
   315  return err;
   316  }
   317  
   318  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   319  if (!priv)
   320  return -ENOMEM;
   321  
   322  priv->base = base;
   323  priv->regmap = dev_get_regmap(dev->parent, NULL);
   324  if (IS_ERR(priv->regmap)) {
   325  err = PTR_ERR(priv->regmap);
   326  dev_err(dev, "failed to get regmap: %d\n", err);
   327  return err;
   328  }
   329  
   330  for_each_child_of_node(np, child) {
   331  err = of_property_read_u32(child, "reg", );
   332  if (err) {
   333  of_node_put(child);
   334  return err;
   335  }
   336  
 > 337  if (reg < 0 || reg >= SC27XX_LEDS_MAX
   338  || priv->leds[reg].active) {
   339  of_node_put(child);
   340  return -EINVAL;
   341  }
   342  
   343  priv->leds[reg].active = true;
   344  priv->leds[reg].ldev.name =
   345  of_get_property(child, "label", NULL) ? : 
child->name;
   346  }
   347  
   348  return sc27xx_led_register(dev, priv);
   349  }
   350  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver

[kbuild test robot]

Hi Xiaotong,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on robh/for-next]
[also build test WARNING on v4.17-rc3 next-20180504]
[cannot apply to j.anaszewski-leds/for-next]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Baolin-Wang/dt-bindings-leds-Add-SC27xx-breathing-light-controller-documentation/20180504-200830
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next

smatch warnings:
drivers/leds/leds-sc27xx-bltc.c:337 sc27xx_led_probe() warn: unsigned 'reg' is 
never less than zero.

vim +/reg +337 drivers/leds/leds-sc27xx-bltc.c

   299  
   300  static int sc27xx_led_probe(struct platform_device *pdev)
   301  {
   302  struct device *dev = >dev;
   303  struct device_node *np = dev->of_node, *child;
   304  struct sc27xx_led_priv *priv;
   305  u32 base, count, reg;
   306  int err;
   307  
   308  count = of_get_child_count(np);
   309  if (!count || count > SC27XX_LEDS_MAX)
   310  return -EINVAL;
   311  
   312  err = of_property_read_u32(np, "reg", );
   313  if (err) {
   314  dev_err(dev, "fail to get reg of property\n");
   315  return err;
   316  }
   317  
   318  priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
   319  if (!priv)
   320  return -ENOMEM;
   321  
   322  priv->base = base;
   323  priv->regmap = dev_get_regmap(dev->parent, NULL);
   324  if (IS_ERR(priv->regmap)) {
   325  err = PTR_ERR(priv->regmap);
   326  dev_err(dev, "failed to get regmap: %d\n", err);
   327  return err;
   328  }
   329  
   330  for_each_child_of_node(np, child) {
   331  err = of_property_read_u32(child, "reg", );
   332  if (err) {
   333  of_node_put(child);
   334  return err;
   335  }
   336  
 > 337  if (reg < 0 || reg >= SC27XX_LEDS_MAX
   338  || priv->leds[reg].active) {
   339  of_node_put(child);
   340  return -EINVAL;
   341  }
   342  
   343  priv->leds[reg].active = true;
   344  priv->leds[reg].ldev.name =
   345  of_get_property(child, "label", NULL) ? : 
child->name;
   346  }
   347  
   348  return sc27xx_led_register(dev, priv);
   349  }
   350  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation

Re: [Ksummit-discuss] bug-introducing patches

[Sasha Levin]

On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
>On Fri, May 04, 2018 at 09:51:14PM +, Sasha Levin wrote:
>> I don't have an objection to moving this to it's own tag. It will make
>> my scripts somewhat simpler for sure.
>
>It's not a matter "moving this it's own tag", but creating a new tag
>--- because what is in the docs is a lie.  It does not describe what
>we do today.  And current practice is the reality, not what is in the
>docs.

I'm really confused here. What do you mean with "not describe what we do
today"?

The doc allows for three ways to tag a patch:

1. Empty tag: "Cc: sta...@vger.kernel.org"
2. With a version, quoting from the doc:

Also, some patches may have kernel version prerequisites.  This can be
specified in the following format in the sign-off area:

 Cc:  # 3.3.x

The tag has the meaning of:

 git cherry-pick 

For each "-stable" tree starting with the specified version.

3. With a prereq commit, which is in the form of:

Cc:  # 3.3.x: a1f84a3: sched: Check for idle

We expect this to be used rarely used, and indeed it's not used as much.

>As to whether we should create a new tag to support explicit
>dependencies, I'll leave that between you and Greg K-H and the rest of
>the stable maintainers.  :-)

Re: [Ksummit-discuss] bug-introducing patches

[Sasha Levin]

On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
>On Fri, May 04, 2018 at 09:51:14PM +, Sasha Levin wrote:
>> I don't have an objection to moving this to it's own tag. It will make
>> my scripts somewhat simpler for sure.
>
>It's not a matter "moving this it's own tag", but creating a new tag
>--- because what is in the docs is a lie.  It does not describe what
>we do today.  And current practice is the reality, not what is in the
>docs.

I'm really confused here. What do you mean with "not describe what we do
today"?

The doc allows for three ways to tag a patch:

1. Empty tag: "Cc: sta...@vger.kernel.org"
2. With a version, quoting from the doc:

Also, some patches may have kernel version prerequisites.  This can be
specified in the following format in the sign-off area:

 Cc:  # 3.3.x

The tag has the meaning of:

 git cherry-pick 

For each "-stable" tree starting with the specified version.

3. With a prereq commit, which is in the form of:

Cc:  # 3.3.x: a1f84a3: sched: Check for idle

We expect this to be used rarely used, and indeed it's not used as much.

>As to whether we should create a new tag to support explicit
>dependencies, I'll leave that between you and Greg K-H and the rest of
>the stable maintainers.  :-)

Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore

[Paul E. McKenney]

On Fri, May 04, 2018 at 11:38:37PM -0500, Eric W. Biederman wrote:
> "Paul E. McKenney"  writes:
> 
> > On Fri, May 04, 2018 at 03:08:40PM -0500, Eric W. Biederman wrote:
> >> "Paul E. McKenney"  writes:
> >> 
> >> > On Fri, May 04, 2018 at 02:03:04PM -0500, Eric W. Biederman wrote:
> >> >> "Paul E. McKenney"  writes:
> >> >> 
> >> >> > On Fri, May 04, 2018 at 12:17:20PM -0500, Eric W. Biederman wrote:
> >> >> >> Sebastian Andrzej Siewior  writes:
> >> >> >> 
> >> >> >> > On 2018-05-04 11:59:08 [-0500], Eric W. Biederman wrote:
> >> >> >> >> Sebastian Andrzej Siewior  writes:
> >> >> >> >> > From: Anna-Maria Gleixner 
> >> >> >> > …
> >> >> >> >> > This long-term fix has been made in commit 4abf91047cf 
> >> >> >> >> > ("rtmutex: Make >
> >> >> >> >> > wait_lock irq safe") for different reason.
> >> >> >> >> 
> >> >> >> >> Which tree has this change been made in?  I am not finding the 
> >> >> >> >> commit
> >> >> >> >> you mention above in Linus's tree.
> >> >> >> >
> >> >> >> > I'm sorry, it should have been commit b4abf91047cf ("rtmutex: Make
> >> >> >> > wait_lock irq safe").
> >> >> >> 
> >> >> >> Can you fix that in your patch description and can you also up the
> >> >> >> description of rcu_read_unlock?
> >> >> >> 
> >> >> >> If we don't need to jump through hoops it looks very reasonable to
> >> >> >> remove this unnecessary logic.  But we should fix the description
> >> >> >> in rcu_read_unlock that still says we need these hoops.
> >> >> >
> >> >> > The hoops are still required for rcu_read_lock(), otherwise you
> >> >> > get deadlocks between the scheduler and RCU in PREEMPT=y kernels.
> >> >> > What happens with this patch (if I understand it correctly) is that 
> >> >> > the
> >> >> > signal code now uses a different way of jumping through the hoops.
> >> >> > But the hoops are still jumped through.
> >> >> 
> >> >> The patch changes:
> >> >> 
> >> >> local_irq_disable();
> >> >> rcu_read_lock();
> >> >> spin_lock();
> >> >> rcu_read_unlock();
> >> >> 
> >> >> to:
> >> >> 
> >> >> rcu_read_lock();
> >> >> spin_lock_irq();
> >> >> rcu_read_unlock();
> >> >> 
> >> >> Now that I have a chance to relfect on it the fact that the patern
> >> >> that is being restored does not work is scary.  As the failure has
> >> >> nothing to do with lock ordering and people won't realize what is going
> >> >> on.  Especially since the common rcu modes won't care.
> >> >> 
> >> >> So is it true that taking spin_lock_irq before calling rcu_read_unlock
> >> >> is a problem because of rt_mutex_unlock()?  Or has b4abf91047cf 
> >> >> ("rtmutex: Make
> >> >> wait_lock irq safe") actually fixed that and we can correct the
> >> >> documentation of rcu_read_unlock() ?  And fix __lock_task_sighand?
> >> >
> >> > The problem is that the thing taking the lock might be the scheduler,
> >> > or one of the locks taken while the scheduler's pi and rq locks are
> >> > held.  This occurs only with RCU-preempt.
> >> >
> >> > Here is what can happen:
> >> >
> >> > oA task does rcu_read_lock().
> >> >
> >> > oThat task is preempted.
> >> >
> >> > oThat task stays preempted for a long time, and is therefore
> >> >  priority boosted.  This boosting involves a high-priority RCU
> >> >  kthread creating an rt_mutex, pretending that the preempted task
> >> >  already holds it, and then acquiring it.
> >> >
> >> > oThe task awakens, acquires the scheduler's rq lock, and
> >> >  then does rcu_read_unlock().
> >> >
> >> > oBecause the task has been priority boosted, __rcu_read_unlock()
> >> >  invokes the rcu_read_unlock_special() slowpath, which does
> >> >  (as you say) rt_mutex_unlock() to deboost.  The deboosting
> >> >  can cause the scheduler to acquire the rq and pi locks, which
> >> >  results in deadlock.
> >> >
> >> > In contrast, holding these scheduler locks across the entirety of the
> >> > RCU-preempt read-side critical section is harmless because then the
> >> > critical section cannot be preempted, which means that priority boosting
> >> > cannot happen, which means that there will be no need to deboost at
> >> > rcu_read_unlock() time.
> >> >
> >> > This restriction has not changed, and as far as I can see is inherent
> >> > in the fact that RCU uses the scheduler and the scheduler uses RCU.
> >> > There is going to be an odd corner case in there somewhere!
> >> 
> >> However if I read things correctly b4abf91047cf ("rtmutex: Make
> >> wait_lock irq safe") did change this.
> >> 
> >> In particular it changed things so that it is only the scheduler locks
> >> that matter, not any old lock that disabled interrupts.  This was done
> >> by disabling disabling interrupts when taking the wait_lock.
> >> 
> >> The rcu_read_unlock documentation states:
> >> 
> >>  * In most situations, rcu_read_unlock() is immune from deadlock.
> >>  * 

Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore

[Paul E. McKenney]

On Fri, May 04, 2018 at 11:38:37PM -0500, Eric W. Biederman wrote:
> "Paul E. McKenney"  writes:
> 
> > On Fri, May 04, 2018 at 03:08:40PM -0500, Eric W. Biederman wrote:
> >> "Paul E. McKenney"  writes:
> >> 
> >> > On Fri, May 04, 2018 at 02:03:04PM -0500, Eric W. Biederman wrote:
> >> >> "Paul E. McKenney"  writes:
> >> >> 
> >> >> > On Fri, May 04, 2018 at 12:17:20PM -0500, Eric W. Biederman wrote:
> >> >> >> Sebastian Andrzej Siewior  writes:
> >> >> >> 
> >> >> >> > On 2018-05-04 11:59:08 [-0500], Eric W. Biederman wrote:
> >> >> >> >> Sebastian Andrzej Siewior  writes:
> >> >> >> >> > From: Anna-Maria Gleixner 
> >> >> >> > …
> >> >> >> >> > This long-term fix has been made in commit 4abf91047cf 
> >> >> >> >> > ("rtmutex: Make >
> >> >> >> >> > wait_lock irq safe") for different reason.
> >> >> >> >> 
> >> >> >> >> Which tree has this change been made in?  I am not finding the 
> >> >> >> >> commit
> >> >> >> >> you mention above in Linus's tree.
> >> >> >> >
> >> >> >> > I'm sorry, it should have been commit b4abf91047cf ("rtmutex: Make
> >> >> >> > wait_lock irq safe").
> >> >> >> 
> >> >> >> Can you fix that in your patch description and can you also up the
> >> >> >> description of rcu_read_unlock?
> >> >> >> 
> >> >> >> If we don't need to jump through hoops it looks very reasonable to
> >> >> >> remove this unnecessary logic.  But we should fix the description
> >> >> >> in rcu_read_unlock that still says we need these hoops.
> >> >> >
> >> >> > The hoops are still required for rcu_read_lock(), otherwise you
> >> >> > get deadlocks between the scheduler and RCU in PREEMPT=y kernels.
> >> >> > What happens with this patch (if I understand it correctly) is that 
> >> >> > the
> >> >> > signal code now uses a different way of jumping through the hoops.
> >> >> > But the hoops are still jumped through.
> >> >> 
> >> >> The patch changes:
> >> >> 
> >> >> local_irq_disable();
> >> >> rcu_read_lock();
> >> >> spin_lock();
> >> >> rcu_read_unlock();
> >> >> 
> >> >> to:
> >> >> 
> >> >> rcu_read_lock();
> >> >> spin_lock_irq();
> >> >> rcu_read_unlock();
> >> >> 
> >> >> Now that I have a chance to relfect on it the fact that the patern
> >> >> that is being restored does not work is scary.  As the failure has
> >> >> nothing to do with lock ordering and people won't realize what is going
> >> >> on.  Especially since the common rcu modes won't care.
> >> >> 
> >> >> So is it true that taking spin_lock_irq before calling rcu_read_unlock
> >> >> is a problem because of rt_mutex_unlock()?  Or has b4abf91047cf 
> >> >> ("rtmutex: Make
> >> >> wait_lock irq safe") actually fixed that and we can correct the
> >> >> documentation of rcu_read_unlock() ?  And fix __lock_task_sighand?
> >> >
> >> > The problem is that the thing taking the lock might be the scheduler,
> >> > or one of the locks taken while the scheduler's pi and rq locks are
> >> > held.  This occurs only with RCU-preempt.
> >> >
> >> > Here is what can happen:
> >> >
> >> > oA task does rcu_read_lock().
> >> >
> >> > oThat task is preempted.
> >> >
> >> > oThat task stays preempted for a long time, and is therefore
> >> >  priority boosted.  This boosting involves a high-priority RCU
> >> >  kthread creating an rt_mutex, pretending that the preempted task
> >> >  already holds it, and then acquiring it.
> >> >
> >> > oThe task awakens, acquires the scheduler's rq lock, and
> >> >  then does rcu_read_unlock().
> >> >
> >> > oBecause the task has been priority boosted, __rcu_read_unlock()
> >> >  invokes the rcu_read_unlock_special() slowpath, which does
> >> >  (as you say) rt_mutex_unlock() to deboost.  The deboosting
> >> >  can cause the scheduler to acquire the rq and pi locks, which
> >> >  results in deadlock.
> >> >
> >> > In contrast, holding these scheduler locks across the entirety of the
> >> > RCU-preempt read-side critical section is harmless because then the
> >> > critical section cannot be preempted, which means that priority boosting
> >> > cannot happen, which means that there will be no need to deboost at
> >> > rcu_read_unlock() time.
> >> >
> >> > This restriction has not changed, and as far as I can see is inherent
> >> > in the fact that RCU uses the scheduler and the scheduler uses RCU.
> >> > There is going to be an odd corner case in there somewhere!
> >> 
> >> However if I read things correctly b4abf91047cf ("rtmutex: Make
> >> wait_lock irq safe") did change this.
> >> 
> >> In particular it changed things so that it is only the scheduler locks
> >> that matter, not any old lock that disabled interrupts.  This was done
> >> by disabling disabling interrupts when taking the wait_lock.
> >> 
> >> The rcu_read_unlock documentation states:
> >> 
> >>  * In most situations, rcu_read_unlock() is immune from deadlock.
> >>  * However, in kernels built with CONFIG_RCU_BOOST, rcu_read_unlock()
> >>  * is responsible for deboosting, which it does via rt_mutex_unlock().
> >>  * 

Re: Suggested new user link command

[Eric W. Biederman]

Tony Wallace  writes:

> On 02/05/18 01:35, Bernd Petrovitsch wrote:
>> Hi all!
>>
>> Top-quoting is evil BTW.
>>
>> On Wed, 2018-05-02 at 00:17 +1200, Tony Wallace wrote:
>>> Two issues here:
>>> 1) Use case (which I have)
>>> 2) Permissions
>>>
>>> 1) Use case
>>>
>>> I am trying to build a backup system.  To avoid duplication of files
>>> over multiple backups I take an Md5 check sum of file contents.  Files
>>> with the same sum are hardlinked together.   Files are linked in to a
>>> standard directory structure a new link for each backup that the file is
>>> part of.  When all backups pointing to a file are deleted the reference
>>> count drops to zero and the file is deleted.  We can keep a database of
>>> checksums and there related inode numbers for linking purposes.  So why
>> a) You can store one of the filenames instead of the inode number.
>> b) You can keep an extra directory with a hardlink named as the inode 
>>number (and delete the entries there if the link count drops to 1).
>>
>>> not have some reference copy to link against it would take no extra
>>> space.  Well it doesn't, but it keeps at least one copy of the file on
>> You have a (disk) space problems on an backup system?
>> I don't think so, Tim;-)
>>
>>> disk forever and the reference count never drops to zero.  Using one of
>>> the backup copies to link to (as stored as the reference copy in the
>>> database) will not work as it could be deleted at any time.
>>>
>>> I have seen on stack overflow others wanting to do this also.
>> "Do. Or do not. There is no try." - Yoda
>> SCNR .
>>
>>> 2) Permissions
>>>
>>> To maintain security there are two requirements: 
>>> 2.1) The effective user must have rights to the inode, that is they must
>>> either own it or be root
>>> 2.2) The effective user must have rights file creation rights to the
>>> directory where it is being linked
>> Obviously (und useful). And on a backup system, there is no problem
>> about that (because the backup software probably runs as root anyways
>> because otherwise 2.1) below will limit the deduplication severely).
>>
>> But for a (to be mainlined/accepted) new syscall, one should think
>> about all situations/use cases and not just one.
>>
>> Additionally to the 2 items above, one needs also x-permissions on
>> *all* directories from / to one existing hardlink in the traditional
>> case and such a syscall bypasses that.
>> Think about it: Everyone can write a progrm to try link all inodes from
>>  0 to ~0 to a directory entry and gets all files with restrictions 2.1)
>> and 2.2) from below.
>> ATM it is enough to `chmod o= ~` to keep all others from all files in
>> my $HOME. Afterwards it's no longer that easy.
>>
>>> If you say no, that is fine, but I do think this idea has merit and can
>>> be done without compromising the system.
>> I'm no one to say no (or yes;-) here to anything;-) I'm just thinking
>> about the implications.
>>
>> And you can always implement a patch and if it's ignored/not accepted,
>> you can use it locally anyways - no one can stop that:-)
>>
>> One more - more constructive - thing: Perhaps it is more
>> acceptable/useful if there is a mount option which must be activated on
>> the backup filesystems and that is not activated anywhere else.
>>
>> MfG,
>>  Bernd
>
> I want to thank everyone for their time. I have taken note of your
> comments.  I believe that there is the need for a companion command
> istat that obtains the stat data from an inode.  Istat may be useful in
> constructing ilink.  For my proposed use case complexity is minimised,
> and effectiveness is maximised by making both istat and ilink root only
> system calls, and then doing my backup as root.  I do not know how a
> mount option would work, and for my own use it is again probably
> unnecessary complexity, but accept it may be necessary if released more
> generally.
>
> I will be dropping the matter now, at least until I have some code to
> show, but if anyone has any more thoughts feel free to drop me an
> email. 

Actually the functionality you are looking for has in some sense already
been implemented, and in a way that does not assume a strictly posix
filesystem.

The system calls are:
name_to_handle_at
open_by_handle_at

Good luck,
Eric

Re: Suggested new user link command

[Eric W. Biederman]

Tony Wallace  writes:

> On 02/05/18 01:35, Bernd Petrovitsch wrote:
>> Hi all!
>>
>> Top-quoting is evil BTW.
>>
>> On Wed, 2018-05-02 at 00:17 +1200, Tony Wallace wrote:
>>> Two issues here:
>>> 1) Use case (which I have)
>>> 2) Permissions
>>>
>>> 1) Use case
>>>
>>> I am trying to build a backup system.  To avoid duplication of files
>>> over multiple backups I take an Md5 check sum of file contents.  Files
>>> with the same sum are hardlinked together.   Files are linked in to a
>>> standard directory structure a new link for each backup that the file is
>>> part of.  When all backups pointing to a file are deleted the reference
>>> count drops to zero and the file is deleted.  We can keep a database of
>>> checksums and there related inode numbers for linking purposes.  So why
>> a) You can store one of the filenames instead of the inode number.
>> b) You can keep an extra directory with a hardlink named as the inode 
>>number (and delete the entries there if the link count drops to 1).
>>
>>> not have some reference copy to link against it would take no extra
>>> space.  Well it doesn't, but it keeps at least one copy of the file on
>> You have a (disk) space problems on an backup system?
>> I don't think so, Tim;-)
>>
>>> disk forever and the reference count never drops to zero.  Using one of
>>> the backup copies to link to (as stored as the reference copy in the
>>> database) will not work as it could be deleted at any time.
>>>
>>> I have seen on stack overflow others wanting to do this also.
>> "Do. Or do not. There is no try." - Yoda
>> SCNR .
>>
>>> 2) Permissions
>>>
>>> To maintain security there are two requirements: 
>>> 2.1) The effective user must have rights to the inode, that is they must
>>> either own it or be root
>>> 2.2) The effective user must have rights file creation rights to the
>>> directory where it is being linked
>> Obviously (und useful). And on a backup system, there is no problem
>> about that (because the backup software probably runs as root anyways
>> because otherwise 2.1) below will limit the deduplication severely).
>>
>> But for a (to be mainlined/accepted) new syscall, one should think
>> about all situations/use cases and not just one.
>>
>> Additionally to the 2 items above, one needs also x-permissions on
>> *all* directories from / to one existing hardlink in the traditional
>> case and such a syscall bypasses that.
>> Think about it: Everyone can write a progrm to try link all inodes from
>>  0 to ~0 to a directory entry and gets all files with restrictions 2.1)
>> and 2.2) from below.
>> ATM it is enough to `chmod o= ~` to keep all others from all files in
>> my $HOME. Afterwards it's no longer that easy.
>>
>>> If you say no, that is fine, but I do think this idea has merit and can
>>> be done without compromising the system.
>> I'm no one to say no (or yes;-) here to anything;-) I'm just thinking
>> about the implications.
>>
>> And you can always implement a patch and if it's ignored/not accepted,
>> you can use it locally anyways - no one can stop that:-)
>>
>> One more - more constructive - thing: Perhaps it is more
>> acceptable/useful if there is a mount option which must be activated on
>> the backup filesystems and that is not activated anywhere else.
>>
>> MfG,
>>  Bernd
>
> I want to thank everyone for their time. I have taken note of your
> comments.  I believe that there is the need for a companion command
> istat that obtains the stat data from an inode.  Istat may be useful in
> constructing ilink.  For my proposed use case complexity is minimised,
> and effectiveness is maximised by making both istat and ilink root only
> system calls, and then doing my backup as root.  I do not know how a
> mount option would work, and for my own use it is again probably
> unnecessary complexity, but accept it may be necessary if released more
> generally.
>
> I will be dropping the matter now, at least until I have some code to
> show, but if anyone has any more thoughts feel free to drop me an
> email. 

Actually the functionality you are looking for has in some sense already
been implemented, and in a way that does not assume a strictly posix
filesystem.

The system calls are:
name_to_handle_at
open_by_handle_at

Good luck,
Eric

Re: [Ksummit-discuss] bug-introducing patches

[Eric W. Biederman]

Willy Tarreau  writes:

> On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
>> On Fri, May 04, 2018 at 09:51:14PM +, Sasha Levin wrote:
>> > I don't have an objection to moving this to it's own tag. It will make
>> > my scripts somewhat simpler for sure.
>> 
>> It's not a matter "moving this it's own tag", but creating a new tag
>> --- because what is in the docs is a lie.  It does not describe what
>> we do today.  And current practice is the reality, not what is in the
>> docs.
>> 
>> As to whether we should create a new tag to support explicit
>> dependencies, I'll leave that between you and Greg K-H and the rest of
>> the stable maintainers.  :-)
>
> Guys, *personally*, I've sometimes been a bit annoyed by the huge amount
> of irregular extra headers trying to compensate for horribly vague commit
> messages, and I'm pretty sure it pisses off patch authors who don't know
> anymore what to put in their description. We need to keep in mind that
> authors are humans and not machines, and that natural language remains
> the best to explain complex dependencies. I'd prefer to see :
>
> This patch needs to be backported to all stable branches that contain
> 717d3133 and 207f5b3c (that's 3.10+) or their respective backports but
> must be adapted (contact me) if only a backport of 717d3133 is present.
>
> Cc: stable # v3.10+
>
> Rather than horrible stuff like this :
>
> Cc: stable # v3.10+ (717d3133 && 207f5b3c) || WARN_ON(back(717d3133))
>
> Of course it's a bit made up, but not too far from what is being discussed
> here, probably only the next step. People will often get complex rules
> wrong, both on the producer and on the consumer side. The day we need a
> compiler to emit commit messages, we'll have to wonder if we didn't go
> too far.
>
> Also I've found the Fixes header pretty useful. It allows patch authors
> to mention what is being fixed without necessarily copying stable,
> because sometimes you'd rather not see your patch immediately backported
> or you think the risks are higher than the bug. And here as well, it's
> only suited for simple situations with a single commit ID, complex
> desriptions have to be part of the commit message body.
>
> I think that what we have now works pretty well but that some descriptions
> lack a bit of detail, especially on the impact of the bug which would help
> decide to backport or drop. This is understandable because often the person
> fixing a bug documents it for people knowing the same subsystem well. But
> when you backport fixes into other kernel versions, you don't know well
> how each subsystem works, and guessing the impact of a bug is not always
> obvious. Most of the time, authors who add Fixes: and/or Cc: stable take
> care of providing enough information, though I'd suspect that sometimes
> they're making efforts trying to figure how to place the information
> there and possibly try to avoid redundancy by writing a shorter body.
>
> At this point, I'm really not seeing what we're trying to improve or
> optimize, and to be honest this discussion worries me a bit, by just
> thinking that it could result in annoying changes...

So the way I use headers today is:
Cc: sta...@vger.kernel.org
Fixes: sha1hash "commit subject"

I will use "Fixes: v2.0.1" if something is so old that it isn't in git.
If it was in bitkeeper and now in tglx's tree I will use:
History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

Just because you won't find the commit in Linus's git tree.

I tend not to find particularly serious bugs, just ancient ones so I
generally figure if it doesn't backport easily it probably is not a
candidate for stable.  The bug has existed for ages without anyone
really carring anyway.

Eric

Re: [Ksummit-discuss] bug-introducing patches

[Eric W. Biederman]

Willy Tarreau  writes:

> On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
>> On Fri, May 04, 2018 at 09:51:14PM +, Sasha Levin wrote:
>> > I don't have an objection to moving this to it's own tag. It will make
>> > my scripts somewhat simpler for sure.
>> 
>> It's not a matter "moving this it's own tag", but creating a new tag
>> --- because what is in the docs is a lie.  It does not describe what
>> we do today.  And current practice is the reality, not what is in the
>> docs.
>> 
>> As to whether we should create a new tag to support explicit
>> dependencies, I'll leave that between you and Greg K-H and the rest of
>> the stable maintainers.  :-)
>
> Guys, *personally*, I've sometimes been a bit annoyed by the huge amount
> of irregular extra headers trying to compensate for horribly vague commit
> messages, and I'm pretty sure it pisses off patch authors who don't know
> anymore what to put in their description. We need to keep in mind that
> authors are humans and not machines, and that natural language remains
> the best to explain complex dependencies. I'd prefer to see :
>
> This patch needs to be backported to all stable branches that contain
> 717d3133 and 207f5b3c (that's 3.10+) or their respective backports but
> must be adapted (contact me) if only a backport of 717d3133 is present.
>
> Cc: stable # v3.10+
>
> Rather than horrible stuff like this :
>
> Cc: stable # v3.10+ (717d3133 && 207f5b3c) || WARN_ON(back(717d3133))
>
> Of course it's a bit made up, but not too far from what is being discussed
> here, probably only the next step. People will often get complex rules
> wrong, both on the producer and on the consumer side. The day we need a
> compiler to emit commit messages, we'll have to wonder if we didn't go
> too far.
>
> Also I've found the Fixes header pretty useful. It allows patch authors
> to mention what is being fixed without necessarily copying stable,
> because sometimes you'd rather not see your patch immediately backported
> or you think the risks are higher than the bug. And here as well, it's
> only suited for simple situations with a single commit ID, complex
> desriptions have to be part of the commit message body.
>
> I think that what we have now works pretty well but that some descriptions
> lack a bit of detail, especially on the impact of the bug which would help
> decide to backport or drop. This is understandable because often the person
> fixing a bug documents it for people knowing the same subsystem well. But
> when you backport fixes into other kernel versions, you don't know well
> how each subsystem works, and guessing the impact of a bug is not always
> obvious. Most of the time, authors who add Fixes: and/or Cc: stable take
> care of providing enough information, though I'd suspect that sometimes
> they're making efforts trying to figure how to place the information
> there and possibly try to avoid redundancy by writing a shorter body.
>
> At this point, I'm really not seeing what we're trying to improve or
> optimize, and to be honest this discussion worries me a bit, by just
> thinking that it could result in annoying changes...

So the way I use headers today is:
Cc: sta...@vger.kernel.org
Fixes: sha1hash "commit subject"

I will use "Fixes: v2.0.1" if something is so old that it isn't in git.
If it was in bitkeeper and now in tglx's tree I will use:
History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git

Just because you won't find the commit in Linus's git tree.

I tend not to find particularly serious bugs, just ancient ones so I
generally figure if it doesn't backport easily it probably is not a
candidate for stable.  The bug has existed for ages without anyone
really carring anyway.

Eric

Re: [PATCH v2 net-next 1/4] umh: introduce fork_usermode_blob() helper

[Jann Horn]

On Thu, May 3, 2018 at 12:36 AM, Alexei Starovoitov  wrote:
> Introduce helper:
> int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
> struct umh_info {
>struct file *pipe_to_umh;
>struct file *pipe_from_umh;
>pid_t pid;
> };
>
> that GPLed kernel modules (signed or unsigned) can use it to execute part
> of its own data as swappable user mode process.
>
> The kernel will do:
> - mount "tmpfs"
> - allocate a unique file in tmpfs
> - populate that file with [data, data + len] bytes
> - user-mode-helper code will do_execve that file and, before the process
>   starts, the kernel will create two unix pipes for bidirectional
>   communication between kernel module and umh
> - close tmpfs file, effectively deleting it
> - the fork_usermode_blob will return zero on success and populate
>   'struct umh_info' with two unix pipes and the pid of the user process
>
> As the first step in the development of the bpfilter project
> the fork_usermode_blob() helper is introduced to allow user mode code
> to be invoked from a kernel module. The idea is that user mode code plus
> normal kernel module code are built as part of the kernel build
> and installed as traditional kernel module into distro specified location,
> such that from a distribution point of view, there is
> no difference between regular kernel modules and kernel modules + umh code.
> Such modules can be signed, modprobed, rmmod, etc. The use of this new helper
> by a kernel module doesn't make it any special from kernel and user space
> tooling point of view.
[...]
> +static struct vfsmount *umh_fs;
> +
> +static int init_tmpfs(void)
> +{
> +   struct file_system_type *type;
> +
> +   if (umh_fs)
> +   return 0;
> +   type = get_fs_type("tmpfs");
> +   if (!type)
> +   return -ENODEV;
> +   umh_fs = kern_mount(type);
> +   if (IS_ERR(umh_fs)) {
> +   int err = PTR_ERR(umh_fs);
> +
> +   put_filesystem(type);
> +   umh_fs = NULL;
> +   return err;
> +   }
> +   return 0;
> +}

Should init_tmpfs() be holding some sort of mutex if it's fiddling
with `umh_fs`? The current code only calls it in initcall context, but
if that ever changes and two processes try to initialize the tmpfs at
the same time, a few things could go wrong.
I guess Luis' suggestion (putting a call to init_tmpfs() in
do_basic_setup()) might be the easiest way to get rid of that problem.

> +static int alloc_tmpfs_file(size_t size, struct file **filp)
> +{
> +   struct file *file;
> +   int err;
> +
> +   err = init_tmpfs();
> +   if (err)
> +   return err;
> +   file = shmem_file_setup_with_mnt(umh_fs, "umh", size, VM_NORESERVE);
> +   if (IS_ERR(file))
> +   return PTR_ERR(file);
> +   *filp = file;
> +   return 0;
> +}

Re: [PATCH v2 net-next 1/4] umh: introduce fork_usermode_blob() helper

[Jann Horn]

On Thu, May 3, 2018 at 12:36 AM, Alexei Starovoitov  wrote:
> Introduce helper:
> int fork_usermode_blob(void *data, size_t len, struct umh_info *info);
> struct umh_info {
>struct file *pipe_to_umh;
>struct file *pipe_from_umh;
>pid_t pid;
> };
>
> that GPLed kernel modules (signed or unsigned) can use it to execute part
> of its own data as swappable user mode process.
>
> The kernel will do:
> - mount "tmpfs"
> - allocate a unique file in tmpfs
> - populate that file with [data, data + len] bytes
> - user-mode-helper code will do_execve that file and, before the process
>   starts, the kernel will create two unix pipes for bidirectional
>   communication between kernel module and umh
> - close tmpfs file, effectively deleting it
> - the fork_usermode_blob will return zero on success and populate
>   'struct umh_info' with two unix pipes and the pid of the user process
>
> As the first step in the development of the bpfilter project
> the fork_usermode_blob() helper is introduced to allow user mode code
> to be invoked from a kernel module. The idea is that user mode code plus
> normal kernel module code are built as part of the kernel build
> and installed as traditional kernel module into distro specified location,
> such that from a distribution point of view, there is
> no difference between regular kernel modules and kernel modules + umh code.
> Such modules can be signed, modprobed, rmmod, etc. The use of this new helper
> by a kernel module doesn't make it any special from kernel and user space
> tooling point of view.
[...]
> +static struct vfsmount *umh_fs;
> +
> +static int init_tmpfs(void)
> +{
> +   struct file_system_type *type;
> +
> +   if (umh_fs)
> +   return 0;
> +   type = get_fs_type("tmpfs");
> +   if (!type)
> +   return -ENODEV;
> +   umh_fs = kern_mount(type);
> +   if (IS_ERR(umh_fs)) {
> +   int err = PTR_ERR(umh_fs);
> +
> +   put_filesystem(type);
> +   umh_fs = NULL;
> +   return err;
> +   }
> +   return 0;
> +}

Should init_tmpfs() be holding some sort of mutex if it's fiddling
with `umh_fs`? The current code only calls it in initcall context, but
if that ever changes and two processes try to initialize the tmpfs at
the same time, a few things could go wrong.
I guess Luis' suggestion (putting a call to init_tmpfs() in
do_basic_setup()) might be the easiest way to get rid of that problem.

> +static int alloc_tmpfs_file(size_t size, struct file **filp)
> +{
> +   struct file *file;
> +   int err;
> +
> +   err = init_tmpfs();
> +   if (err)
> +   return err;
> +   file = shmem_file_setup_with_mnt(umh_fs, "umh", size, VM_NORESERVE);
> +   if (IS_ERR(file))
> +   return PTR_ERR(file);
> +   *filp = file;
> +   return 0;
> +}

Re: [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey

[Dave Hansen]

On 05/04/2018 06:12 PM, Ram Pai wrote:
>> That new line boils down to:
>>
>>  [ilog2(0)]  = "",
>>
>> on x86.  It wasn't *obvious* to me that it is OK to do that.  The other
>> possibly undefined bits (VM_SOFTDIRTY for instance) #ifdef themselves
>> out of this array.
>>
>> I would just be a wee bit worried that this would overwrite the 0 entry
>> ("??") with "".
> Yes it would :-( and could potentially break anything that depends on
> 0th entry being "??"
> 
> Is the following fix acceptable?
> 
> #if VM_PKEY_BIT4
> [ilog2(VM_PKEY_BIT4)]   = "",
> #endif

Yep, I think that works for me.

Re: [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey

[Dave Hansen]

On 05/04/2018 06:12 PM, Ram Pai wrote:
>> That new line boils down to:
>>
>>  [ilog2(0)]  = "",
>>
>> on x86.  It wasn't *obvious* to me that it is OK to do that.  The other
>> possibly undefined bits (VM_SOFTDIRTY for instance) #ifdef themselves
>> out of this array.
>>
>> I would just be a wee bit worried that this would overwrite the 0 entry
>> ("??") with "".
> Yes it would :-( and could potentially break anything that depends on
> 0th entry being "??"
> 
> Is the following fix acceptable?
> 
> #if VM_PKEY_BIT4
> [ilog2(VM_PKEY_BIT4)]   = "",
> #endif

Yep, I think that works for me.

Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore

[Eric W. Biederman]

"Paul E. McKenney"  writes:

> On Fri, May 04, 2018 at 03:08:40PM -0500, Eric W. Biederman wrote:
>> "Paul E. McKenney"  writes:
>> 
>> > On Fri, May 04, 2018 at 02:03:04PM -0500, Eric W. Biederman wrote:
>> >> "Paul E. McKenney"  writes:
>> >> 
>> >> > On Fri, May 04, 2018 at 12:17:20PM -0500, Eric W. Biederman wrote:
>> >> >> Sebastian Andrzej Siewior  writes:
>> >> >> 
>> >> >> > On 2018-05-04 11:59:08 [-0500], Eric W. Biederman wrote:
>> >> >> >> Sebastian Andrzej Siewior  writes:
>> >> >> >> > From: Anna-Maria Gleixner 
>> >> >> > …
>> >> >> >> > This long-term fix has been made in commit 4abf91047cf ("rtmutex: 
>> >> >> >> > Make >
>> >> >> >> > wait_lock irq safe") for different reason.
>> >> >> >> 
>> >> >> >> Which tree has this change been made in?  I am not finding the 
>> >> >> >> commit
>> >> >> >> you mention above in Linus's tree.
>> >> >> >
>> >> >> > I'm sorry, it should have been commit b4abf91047cf ("rtmutex: Make
>> >> >> > wait_lock irq safe").
>> >> >> 
>> >> >> Can you fix that in your patch description and can you also up the
>> >> >> description of rcu_read_unlock?
>> >> >> 
>> >> >> If we don't need to jump through hoops it looks very reasonable to
>> >> >> remove this unnecessary logic.  But we should fix the description
>> >> >> in rcu_read_unlock that still says we need these hoops.
>> >> >
>> >> > The hoops are still required for rcu_read_lock(), otherwise you
>> >> > get deadlocks between the scheduler and RCU in PREEMPT=y kernels.
>> >> > What happens with this patch (if I understand it correctly) is that the
>> >> > signal code now uses a different way of jumping through the hoops.
>> >> > But the hoops are still jumped through.
>> >> 
>> >> The patch changes:
>> >> 
>> >> local_irq_disable();
>> >> rcu_read_lock();
>> >> spin_lock();
>> >> rcu_read_unlock();
>> >> 
>> >> to:
>> >> 
>> >> rcu_read_lock();
>> >> spin_lock_irq();
>> >> rcu_read_unlock();
>> >> 
>> >> Now that I have a chance to relfect on it the fact that the patern
>> >> that is being restored does not work is scary.  As the failure has
>> >> nothing to do with lock ordering and people won't realize what is going
>> >> on.  Especially since the common rcu modes won't care.
>> >> 
>> >> So is it true that taking spin_lock_irq before calling rcu_read_unlock
>> >> is a problem because of rt_mutex_unlock()?  Or has b4abf91047cf 
>> >> ("rtmutex: Make
>> >> wait_lock irq safe") actually fixed that and we can correct the
>> >> documentation of rcu_read_unlock() ?  And fix __lock_task_sighand?
>> >
>> > The problem is that the thing taking the lock might be the scheduler,
>> > or one of the locks taken while the scheduler's pi and rq locks are
>> > held.  This occurs only with RCU-preempt.
>> >
>> > Here is what can happen:
>> >
>> > o  A task does rcu_read_lock().
>> >
>> > o  That task is preempted.
>> >
>> > o  That task stays preempted for a long time, and is therefore
>> >priority boosted.  This boosting involves a high-priority RCU
>> >kthread creating an rt_mutex, pretending that the preempted task
>> >already holds it, and then acquiring it.
>> >
>> > o  The task awakens, acquires the scheduler's rq lock, and
>> >then does rcu_read_unlock().
>> >
>> > o  Because the task has been priority boosted, __rcu_read_unlock()
>> >invokes the rcu_read_unlock_special() slowpath, which does
>> >(as you say) rt_mutex_unlock() to deboost.  The deboosting
>> >can cause the scheduler to acquire the rq and pi locks, which
>> >results in deadlock.
>> >
>> > In contrast, holding these scheduler locks across the entirety of the
>> > RCU-preempt read-side critical section is harmless because then the
>> > critical section cannot be preempted, which means that priority boosting
>> > cannot happen, which means that there will be no need to deboost at
>> > rcu_read_unlock() time.
>> >
>> > This restriction has not changed, and as far as I can see is inherent
>> > in the fact that RCU uses the scheduler and the scheduler uses RCU.
>> > There is going to be an odd corner case in there somewhere!
>> 
>> However if I read things correctly b4abf91047cf ("rtmutex: Make
>> wait_lock irq safe") did change this.
>> 
>> In particular it changed things so that it is only the scheduler locks
>> that matter, not any old lock that disabled interrupts.  This was done
>> by disabling disabling interrupts when taking the wait_lock.
>> 
>> The rcu_read_unlock documentation states:
>> 
>>  * In most situations, rcu_read_unlock() is immune from deadlock.
>>  * However, in kernels built with CONFIG_RCU_BOOST, rcu_read_unlock()
>>  * is responsible for deboosting, which it does via rt_mutex_unlock().
>>  * Unfortunately, this function acquires the scheduler's runqueue and
>>  * priority-inheritance spinlocks.  This means that deadlock could result
>>  * if the caller 

Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore

[Eric W. Biederman]

"Paul E. McKenney"  writes:

> On Fri, May 04, 2018 at 03:08:40PM -0500, Eric W. Biederman wrote:
>> "Paul E. McKenney"  writes:
>> 
>> > On Fri, May 04, 2018 at 02:03:04PM -0500, Eric W. Biederman wrote:
>> >> "Paul E. McKenney"  writes:
>> >> 
>> >> > On Fri, May 04, 2018 at 12:17:20PM -0500, Eric W. Biederman wrote:
>> >> >> Sebastian Andrzej Siewior  writes:
>> >> >> 
>> >> >> > On 2018-05-04 11:59:08 [-0500], Eric W. Biederman wrote:
>> >> >> >> Sebastian Andrzej Siewior  writes:
>> >> >> >> > From: Anna-Maria Gleixner 
>> >> >> > …
>> >> >> >> > This long-term fix has been made in commit 4abf91047cf ("rtmutex: 
>> >> >> >> > Make >
>> >> >> >> > wait_lock irq safe") for different reason.
>> >> >> >> 
>> >> >> >> Which tree has this change been made in?  I am not finding the 
>> >> >> >> commit
>> >> >> >> you mention above in Linus's tree.
>> >> >> >
>> >> >> > I'm sorry, it should have been commit b4abf91047cf ("rtmutex: Make
>> >> >> > wait_lock irq safe").
>> >> >> 
>> >> >> Can you fix that in your patch description and can you also up the
>> >> >> description of rcu_read_unlock?
>> >> >> 
>> >> >> If we don't need to jump through hoops it looks very reasonable to
>> >> >> remove this unnecessary logic.  But we should fix the description
>> >> >> in rcu_read_unlock that still says we need these hoops.
>> >> >
>> >> > The hoops are still required for rcu_read_lock(), otherwise you
>> >> > get deadlocks between the scheduler and RCU in PREEMPT=y kernels.
>> >> > What happens with this patch (if I understand it correctly) is that the
>> >> > signal code now uses a different way of jumping through the hoops.
>> >> > But the hoops are still jumped through.
>> >> 
>> >> The patch changes:
>> >> 
>> >> local_irq_disable();
>> >> rcu_read_lock();
>> >> spin_lock();
>> >> rcu_read_unlock();
>> >> 
>> >> to:
>> >> 
>> >> rcu_read_lock();
>> >> spin_lock_irq();
>> >> rcu_read_unlock();
>> >> 
>> >> Now that I have a chance to relfect on it the fact that the patern
>> >> that is being restored does not work is scary.  As the failure has
>> >> nothing to do with lock ordering and people won't realize what is going
>> >> on.  Especially since the common rcu modes won't care.
>> >> 
>> >> So is it true that taking spin_lock_irq before calling rcu_read_unlock
>> >> is a problem because of rt_mutex_unlock()?  Or has b4abf91047cf 
>> >> ("rtmutex: Make
>> >> wait_lock irq safe") actually fixed that and we can correct the
>> >> documentation of rcu_read_unlock() ?  And fix __lock_task_sighand?
>> >
>> > The problem is that the thing taking the lock might be the scheduler,
>> > or one of the locks taken while the scheduler's pi and rq locks are
>> > held.  This occurs only with RCU-preempt.
>> >
>> > Here is what can happen:
>> >
>> > o  A task does rcu_read_lock().
>> >
>> > o  That task is preempted.
>> >
>> > o  That task stays preempted for a long time, and is therefore
>> >priority boosted.  This boosting involves a high-priority RCU
>> >kthread creating an rt_mutex, pretending that the preempted task
>> >already holds it, and then acquiring it.
>> >
>> > o  The task awakens, acquires the scheduler's rq lock, and
>> >then does rcu_read_unlock().
>> >
>> > o  Because the task has been priority boosted, __rcu_read_unlock()
>> >invokes the rcu_read_unlock_special() slowpath, which does
>> >(as you say) rt_mutex_unlock() to deboost.  The deboosting
>> >can cause the scheduler to acquire the rq and pi locks, which
>> >results in deadlock.
>> >
>> > In contrast, holding these scheduler locks across the entirety of the
>> > RCU-preempt read-side critical section is harmless because then the
>> > critical section cannot be preempted, which means that priority boosting
>> > cannot happen, which means that there will be no need to deboost at
>> > rcu_read_unlock() time.
>> >
>> > This restriction has not changed, and as far as I can see is inherent
>> > in the fact that RCU uses the scheduler and the scheduler uses RCU.
>> > There is going to be an odd corner case in there somewhere!
>> 
>> However if I read things correctly b4abf91047cf ("rtmutex: Make
>> wait_lock irq safe") did change this.
>> 
>> In particular it changed things so that it is only the scheduler locks
>> that matter, not any old lock that disabled interrupts.  This was done
>> by disabling disabling interrupts when taking the wait_lock.
>> 
>> The rcu_read_unlock documentation states:
>> 
>>  * In most situations, rcu_read_unlock() is immune from deadlock.
>>  * However, in kernels built with CONFIG_RCU_BOOST, rcu_read_unlock()
>>  * is responsible for deboosting, which it does via rt_mutex_unlock().
>>  * Unfortunately, this function acquires the scheduler's runqueue and
>>  * priority-inheritance spinlocks.  This means that deadlock could result
>>  * if the caller of rcu_read_unlock() already holds one of these locks or
>>  * any lock that is ever acquired while holding them; or any lock which
>>  * can be taken from 

Re: *alloc API changes

[Matthew Wilcox]

On Fri, May 04, 2018 at 08:46:46PM -0700, Matthew Wilcox wrote:
> On Fri, May 04, 2018 at 06:08:23PM -0700, Kees Cook wrote:
> > The number of permutations for our various allocation function is
> > rather huge. Currently, it is:
> > 
> > system or wrapper:
> > kmem_cache_alloc, kmalloc, vmalloc, kvmalloc, devm_kmalloc,
> > dma_alloc_coherent, pci_alloc_consistent, kmem_alloc, f2fs_kvalloc,
> > and probably others I haven't found yet.
> 
> dma_pool_alloc, page_frag_alloc, gen_pool_alloc, __alloc_bootmem_node,
> cma_alloc, quicklist_alloc (deprecated), mempool_alloc
> 
> > allocation method (not all available in all APIs):
> > regular (kmalloc), zeroed (kzalloc), array (kmalloc_array), zeroed
> > array (kcalloc)
> 
> ... other initialiser (kmem_cache_alloc)

I meant to say that we have a shocking dearth of foo_realloc() functions.
Instead we have drivers and core parts of the kernel implementing their
own stupid slow alloc-a-new-chunk-of-memory-and-memcpy-from-the-old-then-
free when the allocator can probably do a better job (eg vmalloc may
be able to expand the existing are, and if it can't do that, it can
at least remap the underlying pages; the slab allocator may be able to
resize without growing, eg if you krealloc from 1200 bytes to 2000 bytes,
that's going to come out of the same slab).

So, yeah, adding those increases the API permutations even further.
And don't ask about what happens if you allocate with GFP_DMA then
realloc with GFP_HIGHMEM.

Re: *alloc API changes

[Matthew Wilcox]

On Fri, May 04, 2018 at 08:46:46PM -0700, Matthew Wilcox wrote:
> On Fri, May 04, 2018 at 06:08:23PM -0700, Kees Cook wrote:
> > The number of permutations for our various allocation function is
> > rather huge. Currently, it is:
> > 
> > system or wrapper:
> > kmem_cache_alloc, kmalloc, vmalloc, kvmalloc, devm_kmalloc,
> > dma_alloc_coherent, pci_alloc_consistent, kmem_alloc, f2fs_kvalloc,
> > and probably others I haven't found yet.
> 
> dma_pool_alloc, page_frag_alloc, gen_pool_alloc, __alloc_bootmem_node,
> cma_alloc, quicklist_alloc (deprecated), mempool_alloc
> 
> > allocation method (not all available in all APIs):
> > regular (kmalloc), zeroed (kzalloc), array (kmalloc_array), zeroed
> > array (kcalloc)
> 
> ... other initialiser (kmem_cache_alloc)

I meant to say that we have a shocking dearth of foo_realloc() functions.
Instead we have drivers and core parts of the kernel implementing their
own stupid slow alloc-a-new-chunk-of-memory-and-memcpy-from-the-old-then-
free when the allocator can probably do a better job (eg vmalloc may
be able to expand the existing are, and if it can't do that, it can
at least remap the underlying pages; the slab allocator may be able to
resize without growing, eg if you krealloc from 1200 bytes to 2000 bytes,
that's going to come out of the same slab).

So, yeah, adding those increases the API permutations even further.
And don't ask about what happens if you allocate with GFP_DMA then
realloc with GFP_HIGHMEM.

Re: INFO: task hung in blk_freeze_queue

[Tetsuo Handa]

A patch for this specific report is ready. I don't know whether other
"dup" reports will be solved by this patch. Thus, I "undup" this report.

#syz undup

>From eed54c6ae475860a9c63b37b58f34735e792eef7 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa 
Date: Sat, 5 May 2018 12:59:12 +0900
Subject: [PATCH] block/loop: Add recursion check for LOOP_CHANGE_FD request.

syzbot is reporting hung tasks at blk_freeze_queue() [1]. This is
due to ioctl(loop_fd, LOOP_CHANGE_FD, loop_fd) request which should be
rejected. Fix this by adding same recursion check which is used by
LOOP_SET_FD request.

Signed-off-by: Tetsuo Handa 
Reported-by: syzbot 
Cc: Jens Axboe 
---
 drivers/block/loop.c | 59 
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 5d4e316..cee3c01 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -644,6 +644,34 @@ static void loop_reread_partitions(struct loop_device *lo,
__func__, lo->lo_number, lo->lo_file_name, rc);
 }
 
+static inline int is_loop_device(struct file *file)
+{
+   struct inode *i = file->f_mapping->host;
+
+   return i && S_ISBLK(i->i_mode) && MAJOR(i->i_rdev) == LOOP_MAJOR;
+}
+
+static int check_loop_recursion(struct file *f, struct block_device *bdev)
+{
+   /*
+* FIXME: Traversing on other loop devices without corresponding
+* lo_ctl_mutex is not safe. l->lo_state can become Lo_rundown and
+* l->lo_backing_file can become NULL when raced with LOOP_CLR_FD.
+*/
+   while (is_loop_device(f)) {
+   struct loop_device *l;
+
+   if (f->f_mapping->host->i_bdev == bdev)
+   return -EBUSY;
+
+   l = f->f_mapping->host->i_bdev->bd_disk->private_data;
+   if (l->lo_state == Lo_unbound)
+   return -EINVAL;
+   f = l->lo_backing_file;
+   }
+   return 0;
+}
+
 /*
  * loop_change_fd switched the backing store of a loopback device to
  * a new file. This is useful for operating system installers to free up
@@ -673,6 +701,11 @@ static int loop_change_fd(struct loop_device *lo, struct 
block_device *bdev,
if (!file)
goto out;
 
+   /* Avoid recursion */
+   error = check_loop_recursion(file, bdev);
+   if (error)
+   goto out_putf;
+
inode = file->f_mapping->host;
old_file = lo->lo_backing_file;
 
@@ -706,13 +739,6 @@ static int loop_change_fd(struct loop_device *lo, struct 
block_device *bdev,
return error;
 }
 
-static inline int is_loop_device(struct file *file)
-{
-   struct inode *i = file->f_mapping->host;
-
-   return i && S_ISBLK(i->i_mode) && MAJOR(i->i_rdev) == LOOP_MAJOR;
-}
-
 /* loop sysfs attributes */
 
 static ssize_t loop_attr_show(struct device *dev, char *page,
@@ -877,7 +903,7 @@ static int loop_prepare_queue(struct loop_device *lo)
 static int loop_set_fd(struct loop_device *lo, fmode_t mode,
   struct block_device *bdev, unsigned int arg)
 {
-   struct file *file, *f;
+   struct file *file;
struct inode*inode;
struct address_space *mapping;
int lo_flags = 0;
@@ -897,20 +923,9 @@ static int loop_set_fd(struct loop_device *lo, fmode_t 
mode,
goto out_putf;
 
/* Avoid recursion */
-   f = file;
-   while (is_loop_device(f)) {
-   struct loop_device *l;
-
-   if (f->f_mapping->host->i_bdev == bdev)
-   goto out_putf;
-
-   l = f->f_mapping->host->i_bdev->bd_disk->private_data;
-   if (l->lo_state == Lo_unbound) {
-   error = -EINVAL;
-   goto out_putf;
-   }
-   f = l->lo_backing_file;
-   }
+   error = check_loop_recursion(file, bdev);
+   if (error)
+   goto out_putf;
 
mapping = file->f_mapping;
inode = mapping->host;
-- 
1.8.3.1

Re: INFO: task hung in blk_freeze_queue

[Tetsuo Handa]

A patch for this specific report is ready. I don't know whether other
"dup" reports will be solved by this patch. Thus, I "undup" this report.

#syz undup

>From eed54c6ae475860a9c63b37b58f34735e792eef7 Mon Sep 17 00:00:00 2001
From: Tetsuo Handa 
Date: Sat, 5 May 2018 12:59:12 +0900
Subject: [PATCH] block/loop: Add recursion check for LOOP_CHANGE_FD request.

syzbot is reporting hung tasks at blk_freeze_queue() [1]. This is
due to ioctl(loop_fd, LOOP_CHANGE_FD, loop_fd) request which should be
rejected. Fix this by adding same recursion check which is used by
LOOP_SET_FD request.

Signed-off-by: Tetsuo Handa 
Reported-by: syzbot 
Cc: Jens Axboe 
---
 drivers/block/loop.c | 59 
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 5d4e316..cee3c01 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -644,6 +644,34 @@ static void loop_reread_partitions(struct loop_device *lo,
__func__, lo->lo_number, lo->lo_file_name, rc);
 }
 
+static inline int is_loop_device(struct file *file)
+{
+   struct inode *i = file->f_mapping->host;
+
+   return i && S_ISBLK(i->i_mode) && MAJOR(i->i_rdev) == LOOP_MAJOR;
+}
+
+static int check_loop_recursion(struct file *f, struct block_device *bdev)
+{
+   /*
+* FIXME: Traversing on other loop devices without corresponding
+* lo_ctl_mutex is not safe. l->lo_state can become Lo_rundown and
+* l->lo_backing_file can become NULL when raced with LOOP_CLR_FD.
+*/
+   while (is_loop_device(f)) {
+   struct loop_device *l;
+
+   if (f->f_mapping->host->i_bdev == bdev)
+   return -EBUSY;
+
+   l = f->f_mapping->host->i_bdev->bd_disk->private_data;
+   if (l->lo_state == Lo_unbound)
+   return -EINVAL;
+   f = l->lo_backing_file;
+   }
+   return 0;
+}
+
 /*
  * loop_change_fd switched the backing store of a loopback device to
  * a new file. This is useful for operating system installers to free up
@@ -673,6 +701,11 @@ static int loop_change_fd(struct loop_device *lo, struct 
block_device *bdev,
if (!file)
goto out;
 
+   /* Avoid recursion */
+   error = check_loop_recursion(file, bdev);
+   if (error)
+   goto out_putf;
+
inode = file->f_mapping->host;
old_file = lo->lo_backing_file;
 
@@ -706,13 +739,6 @@ static int loop_change_fd(struct loop_device *lo, struct 
block_device *bdev,
return error;
 }
 
-static inline int is_loop_device(struct file *file)
-{
-   struct inode *i = file->f_mapping->host;
-
-   return i && S_ISBLK(i->i_mode) && MAJOR(i->i_rdev) == LOOP_MAJOR;
-}
-
 /* loop sysfs attributes */
 
 static ssize_t loop_attr_show(struct device *dev, char *page,
@@ -877,7 +903,7 @@ static int loop_prepare_queue(struct loop_device *lo)
 static int loop_set_fd(struct loop_device *lo, fmode_t mode,
   struct block_device *bdev, unsigned int arg)
 {
-   struct file *file, *f;
+   struct file *file;
struct inode*inode;
struct address_space *mapping;
int lo_flags = 0;
@@ -897,20 +923,9 @@ static int loop_set_fd(struct loop_device *lo, fmode_t 
mode,
goto out_putf;
 
/* Avoid recursion */
-   f = file;
-   while (is_loop_device(f)) {
-   struct loop_device *l;
-
-   if (f->f_mapping->host->i_bdev == bdev)
-   goto out_putf;
-
-   l = f->f_mapping->host->i_bdev->bd_disk->private_data;
-   if (l->lo_state == Lo_unbound) {
-   error = -EINVAL;
-   goto out_putf;
-   }
-   f = l->lo_backing_file;
-   }
+   error = check_loop_recursion(file, bdev);
+   if (error)
+   goto out_putf;
 
mapping = file->f_mapping;
inode = mapping->host;
-- 
1.8.3.1

Re: INFO: task hung in blk_freeze_queue

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit:cdface520934 Merge tag 'for_linus_stable' of git://git.ker..
git tree:   upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=136c8ee780
kernel config:  https://syzkaller.appspot.com/x/.config?x=61c12b53c2a25ec4
dashboard link: https://syzkaller.appspot.com/bug?extid=2ab52b8d94df5e2eaa01
compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15afa24780
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16f0771780

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+2ab52b8d94df5e2ea...@syzkaller.appspotmail.com

INFO: task syz-executor148:4500 blocked for more than 120 seconds.
  Not tainted 4.17.0-rc2+ #23
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor148 D16648  4500   4481 0x
Call Trace:
 context_switch kernel/sched/core.c:2848 [inline]
 __schedule+0x801/0x1e30 kernel/sched/core.c:3490
 schedule+0xef/0x430 kernel/sched/core.c:3549
 blk_mq_freeze_queue_wait+0x1ce/0x460 block/blk-mq.c:136
 blk_freeze_queue+0x4a/0x80 block/blk-mq.c:165
 blk_mq_freeze_queue+0x15/0x20 block/blk-mq.c:174
 loop_clr_fd+0x226/0xb80 drivers/block/loop.c:1047
 lo_ioctl+0x642/0x2130 drivers/block/loop.c:1404
 __blkdev_driver_ioctl block/ioctl.c:303 [inline]
 blkdev_ioctl+0x9b6/0x2020 block/ioctl.c:601
 block_ioctl+0xee/0x130 fs/block_dev.c:1877
 vfs_ioctl fs/ioctl.c:46 [inline]
 file_ioctl fs/ioctl.c:500 [inline]
 do_vfs_ioctl+0x1cf/0x16a0 fs/ioctl.c:684
 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
 __do_sys_ioctl fs/ioctl.c:708 [inline]
 __se_sys_ioctl fs/ioctl.c:706 [inline]
 __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x449789
RSP: 002b:7f210fae5da8 EFLAGS: 0297 ORIG_RAX: 0010
RAX: ffda RBX: 006dac3c RCX: 00449789
RDX: 00449789 RSI: 4c01 RDI: 0003
RBP:  R08:  R09: 
R10:  R11: 0297 R12: 006dac38
R13: 0030656c69662f2e R14: 6f6f6c2f7665642f R15: 0007

Showing all locks held in the system:
2 locks held by khungtaskd/893:
 #0: 45f40930 (rcu_read_lock){}, at:  
check_hung_uninterruptible_tasks kernel/hung_task.c:175 [inline]
 #0: 45f40930 (rcu_read_lock){}, at: watchdog+0x1ff/0xf60  
kernel/hung_task.c:249
 #1: 81898718 (tasklist_lock){.+.+}, at:  
debug_show_all_locks+0xde/0x34a kernel/locking/lockdep.c:4470

1 lock held by rsyslogd/4362:
 #0: 2e322c73 (>f_pos_lock){+.+.}, at: __run_timers+0x16e/0xc50  
kernel/time/timer.c:1658

2 locks held by getty/4452:
 #0: 3abe4bd2 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 35e35fb8 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4453:
 #0: 4e78faf9 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 44d079f2 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4454:
 #0: 37bf7fca (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: fc65c2e0 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4455:
 #0: 650b41ff (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: f8a69a89 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4456:
 #0: 33547e18 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 0c85318d (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4457:
 #0: e5cb3908 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 9fc1aed4 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4458:
 #0: 55360c24 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 2bcd4fa8 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

1 lock held by syz-executor148/4486:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  
drivers/block/loop.c:1391

1 lock held by syz-executor148/4500:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  
drivers/block/loop.c:1391

1 lock held by syz-executor148/4514:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  
drivers/block/loop.c:1391

1 lock held by syz-executor148/4515:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  

Re: INFO: task hung in blk_freeze_queue

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit:cdface520934 Merge tag 'for_linus_stable' of git://git.ker..
git tree:   upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=136c8ee780
kernel config:  https://syzkaller.appspot.com/x/.config?x=61c12b53c2a25ec4
dashboard link: https://syzkaller.appspot.com/bug?extid=2ab52b8d94df5e2eaa01
compiler:   gcc (GCC) 8.0.1 20180413 (experimental)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=15afa24780
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=16f0771780

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+2ab52b8d94df5e2ea...@syzkaller.appspotmail.com

INFO: task syz-executor148:4500 blocked for more than 120 seconds.
  Not tainted 4.17.0-rc2+ #23
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
syz-executor148 D16648  4500   4481 0x
Call Trace:
 context_switch kernel/sched/core.c:2848 [inline]
 __schedule+0x801/0x1e30 kernel/sched/core.c:3490
 schedule+0xef/0x430 kernel/sched/core.c:3549
 blk_mq_freeze_queue_wait+0x1ce/0x460 block/blk-mq.c:136
 blk_freeze_queue+0x4a/0x80 block/blk-mq.c:165
 blk_mq_freeze_queue+0x15/0x20 block/blk-mq.c:174
 loop_clr_fd+0x226/0xb80 drivers/block/loop.c:1047
 lo_ioctl+0x642/0x2130 drivers/block/loop.c:1404
 __blkdev_driver_ioctl block/ioctl.c:303 [inline]
 blkdev_ioctl+0x9b6/0x2020 block/ioctl.c:601
 block_ioctl+0xee/0x130 fs/block_dev.c:1877
 vfs_ioctl fs/ioctl.c:46 [inline]
 file_ioctl fs/ioctl.c:500 [inline]
 do_vfs_ioctl+0x1cf/0x16a0 fs/ioctl.c:684
 ksys_ioctl+0xa9/0xd0 fs/ioctl.c:701
 __do_sys_ioctl fs/ioctl.c:708 [inline]
 __se_sys_ioctl fs/ioctl.c:706 [inline]
 __x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:706
 do_syscall_64+0x1b1/0x800 arch/x86/entry/common.c:287
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x449789
RSP: 002b:7f210fae5da8 EFLAGS: 0297 ORIG_RAX: 0010
RAX: ffda RBX: 006dac3c RCX: 00449789
RDX: 00449789 RSI: 4c01 RDI: 0003
RBP:  R08:  R09: 
R10:  R11: 0297 R12: 006dac38
R13: 0030656c69662f2e R14: 6f6f6c2f7665642f R15: 0007

Showing all locks held in the system:
2 locks held by khungtaskd/893:
 #0: 45f40930 (rcu_read_lock){}, at:  
check_hung_uninterruptible_tasks kernel/hung_task.c:175 [inline]
 #0: 45f40930 (rcu_read_lock){}, at: watchdog+0x1ff/0xf60  
kernel/hung_task.c:249
 #1: 81898718 (tasklist_lock){.+.+}, at:  
debug_show_all_locks+0xde/0x34a kernel/locking/lockdep.c:4470

1 lock held by rsyslogd/4362:
 #0: 2e322c73 (>f_pos_lock){+.+.}, at: __run_timers+0x16e/0xc50  
kernel/time/timer.c:1658

2 locks held by getty/4452:
 #0: 3abe4bd2 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 35e35fb8 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4453:
 #0: 4e78faf9 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 44d079f2 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4454:
 #0: 37bf7fca (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: fc65c2e0 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4455:
 #0: 650b41ff (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: f8a69a89 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4456:
 #0: 33547e18 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 0c85318d (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4457:
 #0: e5cb3908 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 9fc1aed4 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

2 locks held by getty/4458:
 #0: 55360c24 (>ldisc_sem){}, at:  
ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365
 #1: 2bcd4fa8 (>atomic_read_lock){+.+.}, at:  
n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131

1 lock held by syz-executor148/4486:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  
drivers/block/loop.c:1391

1 lock held by syz-executor148/4500:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  
drivers/block/loop.c:1391

1 lock held by syz-executor148/4514:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  
drivers/block/loop.c:1391

1 lock held by syz-executor148/4515:
 #0: bf14345a (>lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130  

Re: *alloc API changes

[Kees Cook]

On Fri, May 4, 2018 at 8:46 PM, Matthew Wilcox  wrote:
> and if you're counting f2fs_*alloc, there's a metric tonne of *alloc
> wrappers out there.

Yeah. *sob*

> That's a little revisionist ;-)  We had kmalloc before we had the slab
> allocator (kernel 1.2, I think?).  But I see your point, and that's
> certainly how it's implemented these days.

Okay, yes, that's true. I did think of that briefly. :)

> I got shot down for proposing adding
> #define malloc(x) kmalloc(x, GFP_KERNEL)
> on the grounds that driver writers will then use malloc in interrupt
> context.  So I think our base version has to be foo_alloc(size, gfp_t).

Okay, fair enough.

> Right, I was thinking:
>
> static inline size_t mul_ab(size_t a, size_t b)
> {
> #if COMPILER_SUPPORTS_OVERFLOW
> unsigned long c;
> if (__builtin_mul_overflow(a, b, ))
> return SIZE_MAX;
> return c;
> #else
> if (b != 0 && a >= SIZE_MAX / b)
> return SIZE_MAX;
> return a * b;
> #endif
> }

Rasmus, what do you think of a saturating version of your helpers?

The only fear I have with the saturating helpers is that we'll end up
using them in places that don't recognize SIZE_MAX. Like, say:

size = mul(a, b) + 1;

then *poof* size == 0. Now, I'd hope that code would use add(mul(a,
b), 1), but still... it makes me nervous.

> You don't need the size check here.  We have the size check buried deep in
> alloc_pages (look for MAX_ORDER), so kmalloc and then alloc_pages will try
> a bunch of paths all of which fail before returning NULL.

Good point. Though it does kind of creep me out to let a known-bad
size float around in the allocator until it decides to reject it. I
would think an early:

if (unlikely(size == SIZE_MAX))
return NULL;

would have virtually no cycle count difference...

> I'd rather have a mul_ab(), mul_abc(), mul_ab_add_c(), etc. than nest
> calls to mult().

Agreed. I think having exactly those would cover almost everything,
and the two places where a 4-factor product is needed could just nest
them. (bikeshed: the very common mul_ab() should just be mul(), IMO.)

> Nono, Linus had the better proposal, struct_size(p, member, n).

Oh, yes! I totally missed that in the threads.

> Ooh, we could instantiate classes and ... yeah, no, not C++.  We *could*
> abuse the C preprocessor to autogenerate every variant, but I hate that
> because you can't grep for it.

Right, no. I think if we can ditch *calloc() and _array() by using
saturating helpers, we'll have the API in a much better form:

kmalloc(foo * bar, GFP_KERNEL);
into
kmalloc_array(foo, bar, GFP_KERNEL);
into
kmalloc(mul(foo, bar), GFP_KERNEL);

and

kmalloc(foo * bar, GFP_KERNEL | __GFP_ZERO);
into
kzalloc(foo * bar, GFP_KERNEL);
into
kcalloc(foo, bar, GFP_KERNEL);
into
kzalloc(mul(foo, bar), GFP_KERNEL);

and the fun

kzalloc(sizeof(*header) + count * sizeof(*header->element), GFP_KERNEL);
into
kzalloc(struct_size(header, element, count), GFP_KERNEL);

modulo all *alloc* families...

?

-Kees

-- 
Kees Cook
Pixel Security

Re: *alloc API changes

[Kees Cook]

On Fri, May 4, 2018 at 8:46 PM, Matthew Wilcox  wrote:
> and if you're counting f2fs_*alloc, there's a metric tonne of *alloc
> wrappers out there.

Yeah. *sob*

> That's a little revisionist ;-)  We had kmalloc before we had the slab
> allocator (kernel 1.2, I think?).  But I see your point, and that's
> certainly how it's implemented these days.

Okay, yes, that's true. I did think of that briefly. :)

> I got shot down for proposing adding
> #define malloc(x) kmalloc(x, GFP_KERNEL)
> on the grounds that driver writers will then use malloc in interrupt
> context.  So I think our base version has to be foo_alloc(size, gfp_t).

Okay, fair enough.

> Right, I was thinking:
>
> static inline size_t mul_ab(size_t a, size_t b)
> {
> #if COMPILER_SUPPORTS_OVERFLOW
> unsigned long c;
> if (__builtin_mul_overflow(a, b, ))
> return SIZE_MAX;
> return c;
> #else
> if (b != 0 && a >= SIZE_MAX / b)
> return SIZE_MAX;
> return a * b;
> #endif
> }

Rasmus, what do you think of a saturating version of your helpers?

The only fear I have with the saturating helpers is that we'll end up
using them in places that don't recognize SIZE_MAX. Like, say:

size = mul(a, b) + 1;

then *poof* size == 0. Now, I'd hope that code would use add(mul(a,
b), 1), but still... it makes me nervous.

> You don't need the size check here.  We have the size check buried deep in
> alloc_pages (look for MAX_ORDER), so kmalloc and then alloc_pages will try
> a bunch of paths all of which fail before returning NULL.

Good point. Though it does kind of creep me out to let a known-bad
size float around in the allocator until it decides to reject it. I
would think an early:

if (unlikely(size == SIZE_MAX))
return NULL;

would have virtually no cycle count difference...

> I'd rather have a mul_ab(), mul_abc(), mul_ab_add_c(), etc. than nest
> calls to mult().

Agreed. I think having exactly those would cover almost everything,
and the two places where a 4-factor product is needed could just nest
them. (bikeshed: the very common mul_ab() should just be mul(), IMO.)

> Nono, Linus had the better proposal, struct_size(p, member, n).

Oh, yes! I totally missed that in the threads.

> Ooh, we could instantiate classes and ... yeah, no, not C++.  We *could*
> abuse the C preprocessor to autogenerate every variant, but I hate that
> because you can't grep for it.

Right, no. I think if we can ditch *calloc() and _array() by using
saturating helpers, we'll have the API in a much better form:

kmalloc(foo * bar, GFP_KERNEL);
into
kmalloc_array(foo, bar, GFP_KERNEL);
into
kmalloc(mul(foo, bar), GFP_KERNEL);

and

kmalloc(foo * bar, GFP_KERNEL | __GFP_ZERO);
into
kzalloc(foo * bar, GFP_KERNEL);
into
kcalloc(foo, bar, GFP_KERNEL);
into
kzalloc(mul(foo, bar), GFP_KERNEL);

and the fun

kzalloc(sizeof(*header) + count * sizeof(*header->element), GFP_KERNEL);
into
kzalloc(struct_size(header, element, count), GFP_KERNEL);

modulo all *alloc* families...

?

-Kees

-- 
Kees Cook
Pixel Security

Re: [Ksummit-discuss] bug-introducing patches

[Willy Tarreau]

On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
> On Fri, May 04, 2018 at 09:51:14PM +, Sasha Levin wrote:
> > I don't have an objection to moving this to it's own tag. It will make
> > my scripts somewhat simpler for sure.
> 
> It's not a matter "moving this it's own tag", but creating a new tag
> --- because what is in the docs is a lie.  It does not describe what
> we do today.  And current practice is the reality, not what is in the
> docs.
> 
> As to whether we should create a new tag to support explicit
> dependencies, I'll leave that between you and Greg K-H and the rest of
> the stable maintainers.  :-)

Guys, *personally*, I've sometimes been a bit annoyed by the huge amount
of irregular extra headers trying to compensate for horribly vague commit
messages, and I'm pretty sure it pisses off patch authors who don't know
anymore what to put in their description. We need to keep in mind that
authors are humans and not machines, and that natural language remains
the best to explain complex dependencies. I'd prefer to see :

This patch needs to be backported to all stable branches that contain
717d3133 and 207f5b3c (that's 3.10+) or their respective backports but
must be adapted (contact me) if only a backport of 717d3133 is present.

Cc: stable # v3.10+

Rather than horrible stuff like this :

Cc: stable # v3.10+ (717d3133 && 207f5b3c) || WARN_ON(back(717d3133))

Of course it's a bit made up, but not too far from what is being discussed
here, probably only the next step. People will often get complex rules
wrong, both on the producer and on the consumer side. The day we need a
compiler to emit commit messages, we'll have to wonder if we didn't go
too far.

Also I've found the Fixes header pretty useful. It allows patch authors
to mention what is being fixed without necessarily copying stable,
because sometimes you'd rather not see your patch immediately backported
or you think the risks are higher than the bug. And here as well, it's
only suited for simple situations with a single commit ID, complex
desriptions have to be part of the commit message body.

I think that what we have now works pretty well but that some descriptions
lack a bit of detail, especially on the impact of the bug which would help
decide to backport or drop. This is understandable because often the person
fixing a bug documents it for people knowing the same subsystem well. But
when you backport fixes into other kernel versions, you don't know well
how each subsystem works, and guessing the impact of a bug is not always
obvious. Most of the time, authors who add Fixes: and/or Cc: stable take
care of providing enough information, though I'd suspect that sometimes
they're making efforts trying to figure how to place the information
there and possibly try to avoid redundancy by writing a shorter body.

At this point, I'm really not seeing what we're trying to improve or
optimize, and to be honest this discussion worries me a bit, by just
thinking that it could result in annoying changes...

Willy

Re: [Ksummit-discuss] bug-introducing patches

[Willy Tarreau]

On Fri, May 04, 2018 at 07:35:42PM -0400, Theodore Y. Ts'o wrote:
> On Fri, May 04, 2018 at 09:51:14PM +, Sasha Levin wrote:
> > I don't have an objection to moving this to it's own tag. It will make
> > my scripts somewhat simpler for sure.
> 
> It's not a matter "moving this it's own tag", but creating a new tag
> --- because what is in the docs is a lie.  It does not describe what
> we do today.  And current practice is the reality, not what is in the
> docs.
> 
> As to whether we should create a new tag to support explicit
> dependencies, I'll leave that between you and Greg K-H and the rest of
> the stable maintainers.  :-)

Guys, *personally*, I've sometimes been a bit annoyed by the huge amount
of irregular extra headers trying to compensate for horribly vague commit
messages, and I'm pretty sure it pisses off patch authors who don't know
anymore what to put in their description. We need to keep in mind that
authors are humans and not machines, and that natural language remains
the best to explain complex dependencies. I'd prefer to see :

This patch needs to be backported to all stable branches that contain
717d3133 and 207f5b3c (that's 3.10+) or their respective backports but
must be adapted (contact me) if only a backport of 717d3133 is present.

Cc: stable # v3.10+

Rather than horrible stuff like this :

Cc: stable # v3.10+ (717d3133 && 207f5b3c) || WARN_ON(back(717d3133))

Of course it's a bit made up, but not too far from what is being discussed
here, probably only the next step. People will often get complex rules
wrong, both on the producer and on the consumer side. The day we need a
compiler to emit commit messages, we'll have to wonder if we didn't go
too far.

Also I've found the Fixes header pretty useful. It allows patch authors
to mention what is being fixed without necessarily copying stable,
because sometimes you'd rather not see your patch immediately backported
or you think the risks are higher than the bug. And here as well, it's
only suited for simple situations with a single commit ID, complex
desriptions have to be part of the commit message body.

I think that what we have now works pretty well but that some descriptions
lack a bit of detail, especially on the impact of the bug which would help
decide to backport or drop. This is understandable because often the person
fixing a bug documents it for people knowing the same subsystem well. But
when you backport fixes into other kernel versions, you don't know well
how each subsystem works, and guessing the impact of a bug is not always
obvious. Most of the time, authors who add Fixes: and/or Cc: stable take
care of providing enough information, though I'd suspect that sometimes
they're making efforts trying to figure how to place the information
there and possibly try to avoid redundancy by writing a shorter body.

At this point, I'm really not seeing what we're trying to improve or
optimize, and to be honest this discussion worries me a bit, by just
thinking that it could result in annoying changes...

Willy

[PATCH] HID: uhid: fix a missing-check bug

[Wenwen Wang]

In uhid_event_from_user(), if it is in_compat_syscall(), the 'type' of the
event is first fetched from the 'buffer' in userspace and checked. If the
'type' is UHID_CREATE, it is a messed up request with compat pointer, which
could be more than 256 bytes, so it is better allocated from the heap, as
mentioned in the comment. Its fields are then prepared one by one instead
of using a whole copy. For all other cases, the event object is copied
directly from user space. In other words, based on the 'type', the memory
size and structure of the event object vary.

Given that the 'buffer' resides in userspace, a malicious userspace process
can race to change the 'type' between the two copies, which will cause
inconsistency issues, potentially security issues. Plus, various operations
such as uhid_dev_destroy() and uhid_dev_input() are performed based on
'type' in function uhid_char_write(). If 'type' is modified by user, there
could be some issues such as uninitialized uses.

To fix this problem, we need to recheck the type after the second fetch to
make sure it is not UHID_CREATE.

Signed-off-by: Wenwen Wang 
---
 drivers/hid/uhid.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 3c55073..0220385 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -447,11 +447,17 @@ static int uhid_event_from_user(const char __user 
*buffer, size_t len,
event->u.create.country = compat->country;
 
kfree(compat);
-   return 0;
+   } else {
+   if (copy_from_user(event, buffer,
+   min(len, sizeof(*event
+   return -EFAULT;
+   if (event->type == UHID_CREATE)
+   return -EINVAL;
}
-   /* All others can be copied directly */
+   return 0;
}
 
+   /* Others can be copied directly */
if (copy_from_user(event, buffer, min(len, sizeof(*event
return -EFAULT;
 
-- 
2.7.4

[PATCH] HID: uhid: fix a missing-check bug

[Wenwen Wang]

In uhid_event_from_user(), if it is in_compat_syscall(), the 'type' of the
event is first fetched from the 'buffer' in userspace and checked. If the
'type' is UHID_CREATE, it is a messed up request with compat pointer, which
could be more than 256 bytes, so it is better allocated from the heap, as
mentioned in the comment. Its fields are then prepared one by one instead
of using a whole copy. For all other cases, the event object is copied
directly from user space. In other words, based on the 'type', the memory
size and structure of the event object vary.

Given that the 'buffer' resides in userspace, a malicious userspace process
can race to change the 'type' between the two copies, which will cause
inconsistency issues, potentially security issues. Plus, various operations
such as uhid_dev_destroy() and uhid_dev_input() are performed based on
'type' in function uhid_char_write(). If 'type' is modified by user, there
could be some issues such as uninitialized uses.

To fix this problem, we need to recheck the type after the second fetch to
make sure it is not UHID_CREATE.

Signed-off-by: Wenwen Wang 
---
 drivers/hid/uhid.c | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 3c55073..0220385 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -447,11 +447,17 @@ static int uhid_event_from_user(const char __user 
*buffer, size_t len,
event->u.create.country = compat->country;
 
kfree(compat);
-   return 0;
+   } else {
+   if (copy_from_user(event, buffer,
+   min(len, sizeof(*event
+   return -EFAULT;
+   if (event->type == UHID_CREATE)
+   return -EINVAL;
}
-   /* All others can be copied directly */
+   return 0;
}
 
+   /* Others can be copied directly */
if (copy_from_user(event, buffer, min(len, sizeof(*event
return -EFAULT;
 
-- 
2.7.4

Re: [PATCH] percpu_ida: Use _irqsave() instead of local_irq_save() + spin_lock

[Matthew Wilcox]

On Fri, May 04, 2018 at 04:22:16PM -0700, Andrew Morton wrote:
> I'm feeling a bit hostile toward lib/percpu_ida.c in general ;) It has
> very few users and seems rather complicated (what's with that
> schedule() in percpu_ida_alloc?).  I'm suspecting and hoping that if
> someone can figure out what the requirements were, this could all be
> zapped and reimplemented using something else which we already have.

Note that I have no code in percpu_ida ... it's quite different from
the regular IDA.  But I have noticed the stunning similarity between the
percpu_ida and the code in lib/sbitmap.c.  I have no idea which one is
better, but they're essentially doing the same thing.

Re: [PATCH] percpu_ida: Use _irqsave() instead of local_irq_save() + spin_lock

[Matthew Wilcox]

On Fri, May 04, 2018 at 04:22:16PM -0700, Andrew Morton wrote:
> I'm feeling a bit hostile toward lib/percpu_ida.c in general ;) It has
> very few users and seems rather complicated (what's with that
> schedule() in percpu_ida_alloc?).  I'm suspecting and hoping that if
> someone can figure out what the requirements were, this could all be
> zapped and reimplemented using something else which we already have.

Note that I have no code in percpu_ida ... it's quite different from
the regular IDA.  But I have noticed the stunning similarity between the
percpu_ida and the code in lib/sbitmap.c.  I have no idea which one is
better, but they're essentially doing the same thing.

Re: [PATCH v1] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

[Taniya Das]

Yeah sure Stephen.

On 5/5/2018 8:21 AM, Stephen Boyd wrote:

Quoting Taniya Das (2018-05-03 02:09:57)

Hello Stephen,

I have tested the below patch & didn't see any issues.


Alright. Thanks! Can I take that as a "Tested-by"?



--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver

[kbuild test robot]

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ]

url:
https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-panel-Add-Ilitek-ILI9881c-controller-driver/20180505-104031
base:
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c: In function 
'ili9881c_prepare':
>> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:34: warning: 
>> initialization discards 'const' qualifier from pointer target type 
>> [-Wdiscarded-qualifiers]
  struct ili9881c_instr *instr = _init[i];
 ^

vim +/const +303 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c

   284  
   285  static int ili9881c_prepare(struct drm_panel *panel)
   286  {
   287  struct ili9881c *ctx = panel_to_ili9881c(panel);
   288  unsigned int i;
   289  int ret;
   290  
   291  /* Power the panel */
   292  gpiod_set_value(ctx->power, 1);
   293  msleep(5);
   294  
   295  /* And reset it */
   296  gpiod_set_value(ctx->reset, 1);
   297  msleep(20);
   298  
   299  gpiod_set_value(ctx->reset, 0);
   300  msleep(20);
   301  
   302  for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
 > 303  struct ili9881c_instr *instr = _init[i];
   304  
   305  if (instr->op == ILI9881C_SWITCH_PAGE)
   306  ret = ili9881c_switch_page(ctx, 
instr->arg.page);
   307  else if (instr->op == ILI9881C_COMMAND)
   308  ret = ili9881c_send_cmd_data(ctx, 
instr->arg.cmd.cmd,
   309
instr->arg.cmd.data);
   310  
   311  if (ret)
   312  return ret;
   313  }
   314  
   315  ret = ili9881c_switch_page(ctx, 0);
   316  if (ret)
   317  return ret;
   318  
   319  ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, 
MIPI_DSI_DCS_TEAR_MODE_VBLANK);
   320  if (ret)
   321  return ret;
   322  
   323  mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
   324  if (ret)
   325  return ret;
   326  
   327  return 0;
   328  }
   329  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: [PATCH v1] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

[Taniya Das]

Yeah sure Stephen.

On 5/5/2018 8:21 AM, Stephen Boyd wrote:

Quoting Taniya Das (2018-05-03 02:09:57)

Hello Stephen,

I have tested the below patch & didn't see any issues.


Alright. Thanks! Can I take that as a "Tested-by"?



--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation.

--

Re: [PATCH v4 2/3] drm/panel: Add Ilitek ILI9881c panel driver

[kbuild test robot]

Hi Maxime,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ]

url:
https://github.com/0day-ci/linux/commits/Maxime-Ripard/drm-panel-Add-Ilitek-ILI9881c-controller-driver/20180505-104031
base:
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
wget 
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O 
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   drivers/gpu/drm/panel/panel-ilitek-ili9881c.c: In function 
'ili9881c_prepare':
>> drivers/gpu/drm/panel/panel-ilitek-ili9881c.c:303:34: warning: 
>> initialization discards 'const' qualifier from pointer target type 
>> [-Wdiscarded-qualifiers]
  struct ili9881c_instr *instr = _init[i];
 ^

vim +/const +303 drivers/gpu/drm/panel/panel-ilitek-ili9881c.c

   284  
   285  static int ili9881c_prepare(struct drm_panel *panel)
   286  {
   287  struct ili9881c *ctx = panel_to_ili9881c(panel);
   288  unsigned int i;
   289  int ret;
   290  
   291  /* Power the panel */
   292  gpiod_set_value(ctx->power, 1);
   293  msleep(5);
   294  
   295  /* And reset it */
   296  gpiod_set_value(ctx->reset, 1);
   297  msleep(20);
   298  
   299  gpiod_set_value(ctx->reset, 0);
   300  msleep(20);
   301  
   302  for (i = 0; i < ARRAY_SIZE(ili9881c_init); i++) {
 > 303  struct ili9881c_instr *instr = _init[i];
   304  
   305  if (instr->op == ILI9881C_SWITCH_PAGE)
   306  ret = ili9881c_switch_page(ctx, 
instr->arg.page);
   307  else if (instr->op == ILI9881C_COMMAND)
   308  ret = ili9881c_send_cmd_data(ctx, 
instr->arg.cmd.cmd,
   309
instr->arg.cmd.data);
   310  
   311  if (ret)
   312  return ret;
   313  }
   314  
   315  ret = ili9881c_switch_page(ctx, 0);
   316  if (ret)
   317  return ret;
   318  
   319  ret = mipi_dsi_dcs_set_tear_on(ctx->dsi, 
MIPI_DSI_DCS_TEAR_MODE_VBLANK);
   320  if (ret)
   321  return ret;
   322  
   323  mipi_dsi_dcs_exit_sleep_mode(ctx->dsi);
   324  if (ret)
   325  return ret;
   326  
   327  return 0;
   328  }
   329  

---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all   Intel Corporation


.config.gz
Description: application/gzip

Re: *alloc API changes

[Matthew Wilcox]

On Fri, May 04, 2018 at 06:08:23PM -0700, Kees Cook wrote:
> The number of permutations for our various allocation function is
> rather huge. Currently, it is:
> 
> system or wrapper:
> kmem_cache_alloc, kmalloc, vmalloc, kvmalloc, devm_kmalloc,
> dma_alloc_coherent, pci_alloc_consistent, kmem_alloc, f2fs_kvalloc,
> and probably others I haven't found yet.

dma_pool_alloc, page_frag_alloc, gen_pool_alloc, __alloc_bootmem_node,
cma_alloc, quicklist_alloc (deprecated), mempool_alloc

and if you're counting f2fs_*alloc, there's a metric tonne of *alloc
wrappers out there.

> allocation method (not all available in all APIs):
> regular (kmalloc), zeroed (kzalloc), array (kmalloc_array), zeroed
> array (kcalloc)

... other initialiser (kmem_cache_alloc)

> I wonder if we might be able to rearrange our APIs for the general
> case and include a common "kitchen sink" API for the less common
> options. I.e. why do we have an entire set of _node() APIs, 2 sets for
> zeroing (kzalloc, kcalloc), etc.

I'd love it if we had a common pattern for these things.  A regular API
appeals to me (I blame those RISC people in my formative years).

> kmalloc()-family was meant to be a simplification of
> kmem_cache_alloc().

That's a little revisionist ;-)  We had kmalloc before we had the slab
allocator (kernel 1.2, I think?).  But I see your point, and that's
certainly how it's implemented these days.

> vmalloc() duplicated the kmalloc()-family, and
> kvmalloc() does too. Then we have "specialty" allocators (devm, dma,
> pci, f2fs, xfs's kmem) that have subsets and want to perform other
> actions around the base allocators or have their own entirely (e.g.
> dma).
> 
> Instead of all the variations, it seems like we just want a per-family
> alloc() and alloc_attrs(), where alloc_attrs() could handle the less
> common stuff (e.g. gfp, zero, node).
> 
> kmalloc(size, GFP_KERNEL)
> becomes a nice:
> kmalloc(size)

I got shot down for proposing adding
#define malloc(x) kmalloc(x, GFP_KERNEL)
on the grounds that driver writers will then use malloc in interrupt
context.  So I think our base version has to be foo_alloc(size, gfp_t).

> But this doesn't solve the multiplication overflow case at all, which
> is my real goal. Trying to incorporate some of the ideas from other
> threads, maybe we could have a multiplication helper that would
> saturate and the allocator would see that as a signal to return NULL?
> e.g.:
> 
> inline size_t mult(size_t a, size_t b)
> {
> if (b != 0 && a >= SIZE_MAX / b)
> return SIZE_MAX;
> return a * b;
> }
> (really, this kind of helper should be based on Rasmus's helpers which
> do correct type handling)

Right, I was thinking:

static inline size_t mul_ab(size_t a, size_t b)
{
#if COMPILER_SUPPORTS_OVERFLOW
unsigned long c;
if (__builtin_mul_overflow(a, b, ))
return SIZE_MAX;
return c;
#else
if (b != 0 && a >= SIZE_MAX / b)
return SIZE_MAX;
return a * b;
#endif
}

> void *kmalloc(size_t size)
> {
> if (size == SIZE_MAX)
> return NULL;
> kmalloc_attrs(size, GFP_KERNEL, ALLOC_NOZERO, NUMA_NO_NODE);
> }

You don't need the size check here.  We have the size check buried deep in
alloc_pages (look for MAX_ORDER), so kmalloc and then alloc_pages will try
a bunch of paths all of which fail before returning NULL.

> then we get:
> var = kmalloc(mult(num, sizeof(*var)));
> 
> we could drop the *calloc(), *zalloc(), and *_array(), leaving only
> *alloc() and *alloc_attrs() for all the allocator families.
> 
> I honestly can't tell if this is worse than doing all the family
> conversions to *calloc() and *_array() for the 1000ish instances of
> 2-factor products used for size arguments in the *alloc() functions.
> We could still nest them for the 3-factor ones?
> var = kmalloc(multi(row, mult(column, sizeof(*var;
> 
> But now we're just pretending to be LISP.

I'd rather have a mul_ab(), mul_abc(), mul_ab_add_c(), etc. than nest
calls to mult().

> And really, I'd like to keep the nicer *alloc_struct() with all its
> type checking. But then do we do *zalloc_struct(),
> *alloc_struct_node(), etc, etc?

Nono, Linus had the better proposal, struct_size(p, member, n).

> Bleh. C sucks for this.

Ooh, we could instantiate classes and ... yeah, no, not C++.  We *could*
abuse the C preprocessor to autogenerate every variant, but I hate that
because you can't grep for it.

One of the problems with having the single-argument foo_alloc be a static
inline for foo_alloc_attrs is that you then have to marshall four arguments
for the call instead of just one.  I would have two exported symbols for
each variant.

Re: *alloc API changes

[Matthew Wilcox]

On Fri, May 04, 2018 at 06:08:23PM -0700, Kees Cook wrote:
> The number of permutations for our various allocation function is
> rather huge. Currently, it is:
> 
> system or wrapper:
> kmem_cache_alloc, kmalloc, vmalloc, kvmalloc, devm_kmalloc,
> dma_alloc_coherent, pci_alloc_consistent, kmem_alloc, f2fs_kvalloc,
> and probably others I haven't found yet.

dma_pool_alloc, page_frag_alloc, gen_pool_alloc, __alloc_bootmem_node,
cma_alloc, quicklist_alloc (deprecated), mempool_alloc

and if you're counting f2fs_*alloc, there's a metric tonne of *alloc
wrappers out there.

> allocation method (not all available in all APIs):
> regular (kmalloc), zeroed (kzalloc), array (kmalloc_array), zeroed
> array (kcalloc)

... other initialiser (kmem_cache_alloc)

> I wonder if we might be able to rearrange our APIs for the general
> case and include a common "kitchen sink" API for the less common
> options. I.e. why do we have an entire set of _node() APIs, 2 sets for
> zeroing (kzalloc, kcalloc), etc.

I'd love it if we had a common pattern for these things.  A regular API
appeals to me (I blame those RISC people in my formative years).

> kmalloc()-family was meant to be a simplification of
> kmem_cache_alloc().

That's a little revisionist ;-)  We had kmalloc before we had the slab
allocator (kernel 1.2, I think?).  But I see your point, and that's
certainly how it's implemented these days.

> vmalloc() duplicated the kmalloc()-family, and
> kvmalloc() does too. Then we have "specialty" allocators (devm, dma,
> pci, f2fs, xfs's kmem) that have subsets and want to perform other
> actions around the base allocators or have their own entirely (e.g.
> dma).
> 
> Instead of all the variations, it seems like we just want a per-family
> alloc() and alloc_attrs(), where alloc_attrs() could handle the less
> common stuff (e.g. gfp, zero, node).
> 
> kmalloc(size, GFP_KERNEL)
> becomes a nice:
> kmalloc(size)

I got shot down for proposing adding
#define malloc(x) kmalloc(x, GFP_KERNEL)
on the grounds that driver writers will then use malloc in interrupt
context.  So I think our base version has to be foo_alloc(size, gfp_t).

> But this doesn't solve the multiplication overflow case at all, which
> is my real goal. Trying to incorporate some of the ideas from other
> threads, maybe we could have a multiplication helper that would
> saturate and the allocator would see that as a signal to return NULL?
> e.g.:
> 
> inline size_t mult(size_t a, size_t b)
> {
> if (b != 0 && a >= SIZE_MAX / b)
> return SIZE_MAX;
> return a * b;
> }
> (really, this kind of helper should be based on Rasmus's helpers which
> do correct type handling)

Right, I was thinking:

static inline size_t mul_ab(size_t a, size_t b)
{
#if COMPILER_SUPPORTS_OVERFLOW
unsigned long c;
if (__builtin_mul_overflow(a, b, ))
return SIZE_MAX;
return c;
#else
if (b != 0 && a >= SIZE_MAX / b)
return SIZE_MAX;
return a * b;
#endif
}

> void *kmalloc(size_t size)
> {
> if (size == SIZE_MAX)
> return NULL;
> kmalloc_attrs(size, GFP_KERNEL, ALLOC_NOZERO, NUMA_NO_NODE);
> }

You don't need the size check here.  We have the size check buried deep in
alloc_pages (look for MAX_ORDER), so kmalloc and then alloc_pages will try
a bunch of paths all of which fail before returning NULL.

> then we get:
> var = kmalloc(mult(num, sizeof(*var)));
> 
> we could drop the *calloc(), *zalloc(), and *_array(), leaving only
> *alloc() and *alloc_attrs() for all the allocator families.
> 
> I honestly can't tell if this is worse than doing all the family
> conversions to *calloc() and *_array() for the 1000ish instances of
> 2-factor products used for size arguments in the *alloc() functions.
> We could still nest them for the 3-factor ones?
> var = kmalloc(multi(row, mult(column, sizeof(*var;
> 
> But now we're just pretending to be LISP.

I'd rather have a mul_ab(), mul_abc(), mul_ab_add_c(), etc. than nest
calls to mult().

> And really, I'd like to keep the nicer *alloc_struct() with all its
> type checking. But then do we do *zalloc_struct(),
> *alloc_struct_node(), etc, etc?

Nono, Linus had the better proposal, struct_size(p, member, n).

> Bleh. C sucks for this.

Ooh, we could instantiate classes and ... yeah, no, not C++.  We *could*
abuse the C preprocessor to autogenerate every variant, but I hate that
because you can't grep for it.

One of the problems with having the single-argument foo_alloc be a static
inline for foo_alloc_attrs is that you then have to marshall four arguments
for the call instead of just one.  I would have two exported symbols for
each variant.

[PATCH] [v6] staging: wlan-ng: prism2sta: fix indent coding-style issues

[Efstratios Gavas]

Fixed format/style issues found with checkpatch. No code changes.
Corrected alignment of variables after open parenthesis and line breaks.
Checkpatch now returns clean except for "line over 80 char" warnings.

Signed-off-by: Efstratios Gavas 
---
v2(unlabeled): changed title info
v3: improved changelog description
v4: corrected patch versioning
v5: changed SMTP settings. changed maintainer email and removed
upstream contributors per README to eliminate double submission issue
v6: changed SMTP server. changed title. resolved duplicate commit.
---
 drivers/staging/wlan-ng/prism2sta.c | 52 +
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/wlan-ng/prism2sta.c 
b/drivers/staging/wlan-ng/prism2sta.c
index fed0b8ceca6f..914970249680 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -764,16 +764,16 @@ static int prism2sta_getcardinfo(struct wlandevice 
*wlandev)
 
if (hw->cap_sup_sta.id == 0x04) {
netdev_info(wlandev->netdev,
-  "STA:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
-  hw->cap_sup_sta.role, hw->cap_sup_sta.id,
-  hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
-  hw->cap_sup_sta.top);
+   
"STA:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
+   hw->cap_sup_sta.role, hw->cap_sup_sta.id,
+   hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
+   hw->cap_sup_sta.top);
} else {
netdev_info(wlandev->netdev,
-  "AP:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
-  hw->cap_sup_sta.role, hw->cap_sup_sta.id,
-  hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
-  hw->cap_sup_sta.top);
+   
"AP:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
+   hw->cap_sup_sta.role, hw->cap_sup_sta.id,
+   hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
+   hw->cap_sup_sta.top);
}
 
/* Compatibility range, primary f/w actor, CFI supplier */
@@ -1189,7 +1189,6 @@ void prism2sta_processing_defer(struct work_struct *data)
inf = (struct hfa384x_inf_frame *)skb->data;
prism2sta_inf_authreq_defer(wlandev, inf);
}
-
}
 
/* Now let's handle the linkstatus stuff */
@@ -1241,9 +1240,9 @@ void prism2sta_processing_defer(struct work_struct *data)
/* Collect the BSSID, and set state to allow tx */
 
result = hfa384x_drvr_getconfig(hw,
-   HFA384x_RID_CURRENTBSSID,
-   wlandev->bssid,
-   WLAN_BSSID_LEN);
+   
HFA384x_RID_CURRENTBSSID,
+   wlandev->bssid,
+   WLAN_BSSID_LEN);
if (result) {
pr_debug
("getconfig(0x%02x) failed, result = %d\n",
@@ -1260,14 +1259,13 @@ void prism2sta_processing_defer(struct work_struct 
*data)
 HFA384x_RID_CURRENTSSID, result);
return;
}
-   prism2mgmt_bytestr2pstr(
-   (struct hfa384x_bytestr *),
-   (struct p80211pstrd *)>ssid);
+   prism2mgmt_bytestr2pstr((struct hfa384x_bytestr *),
+   (struct p80211pstrd 
*)>ssid);
 
/* Collect the port status */
result = hfa384x_drvr_getconfig16(hw,
-   HFA384x_RID_PORTSTATUS,
-   );
+ 
HFA384x_RID_PORTSTATUS,
+ );
if (result) {
pr_debug
("getconfig(0x%02x) failed, result = %d\n",
@@ -1404,7 +1402,7 @@ void prism2sta_processing_defer(struct work_struct *data)
   ,
   HFA384x_RID_JOINREQUEST_LEN);
netdev_info(wlandev->netdev,
-  "linkstatus=ASSOCFAIL (re-submitting join)\n");
+   "linkstatus=ASSOCFAIL (re-submitting 
join)\n");
} else {
  

[PATCH] [v6] staging: wlan-ng: prism2sta: fix indent coding-style issues

[Efstratios Gavas]

Fixed format/style issues found with checkpatch. No code changes.
Corrected alignment of variables after open parenthesis and line breaks.
Checkpatch now returns clean except for "line over 80 char" warnings.

Signed-off-by: Efstratios Gavas 
---
v2(unlabeled): changed title info
v3: improved changelog description
v4: corrected patch versioning
v5: changed SMTP settings. changed maintainer email and removed
upstream contributors per README to eliminate double submission issue
v6: changed SMTP server. changed title. resolved duplicate commit.
---
 drivers/staging/wlan-ng/prism2sta.c | 52 +
 1 file changed, 24 insertions(+), 28 deletions(-)

diff --git a/drivers/staging/wlan-ng/prism2sta.c 
b/drivers/staging/wlan-ng/prism2sta.c
index fed0b8ceca6f..914970249680 100644
--- a/drivers/staging/wlan-ng/prism2sta.c
+++ b/drivers/staging/wlan-ng/prism2sta.c
@@ -764,16 +764,16 @@ static int prism2sta_getcardinfo(struct wlandevice 
*wlandev)
 
if (hw->cap_sup_sta.id == 0x04) {
netdev_info(wlandev->netdev,
-  "STA:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
-  hw->cap_sup_sta.role, hw->cap_sup_sta.id,
-  hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
-  hw->cap_sup_sta.top);
+   
"STA:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
+   hw->cap_sup_sta.role, hw->cap_sup_sta.id,
+   hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
+   hw->cap_sup_sta.top);
} else {
netdev_info(wlandev->netdev,
-  "AP:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
-  hw->cap_sup_sta.role, hw->cap_sup_sta.id,
-  hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
-  hw->cap_sup_sta.top);
+   
"AP:SUP:role=0x%02x:id=0x%02x:var=0x%02x:b/t=%d/%d\n",
+   hw->cap_sup_sta.role, hw->cap_sup_sta.id,
+   hw->cap_sup_sta.variant, hw->cap_sup_sta.bottom,
+   hw->cap_sup_sta.top);
}
 
/* Compatibility range, primary f/w actor, CFI supplier */
@@ -1189,7 +1189,6 @@ void prism2sta_processing_defer(struct work_struct *data)
inf = (struct hfa384x_inf_frame *)skb->data;
prism2sta_inf_authreq_defer(wlandev, inf);
}
-
}
 
/* Now let's handle the linkstatus stuff */
@@ -1241,9 +1240,9 @@ void prism2sta_processing_defer(struct work_struct *data)
/* Collect the BSSID, and set state to allow tx */
 
result = hfa384x_drvr_getconfig(hw,
-   HFA384x_RID_CURRENTBSSID,
-   wlandev->bssid,
-   WLAN_BSSID_LEN);
+   
HFA384x_RID_CURRENTBSSID,
+   wlandev->bssid,
+   WLAN_BSSID_LEN);
if (result) {
pr_debug
("getconfig(0x%02x) failed, result = %d\n",
@@ -1260,14 +1259,13 @@ void prism2sta_processing_defer(struct work_struct 
*data)
 HFA384x_RID_CURRENTSSID, result);
return;
}
-   prism2mgmt_bytestr2pstr(
-   (struct hfa384x_bytestr *),
-   (struct p80211pstrd *)>ssid);
+   prism2mgmt_bytestr2pstr((struct hfa384x_bytestr *),
+   (struct p80211pstrd 
*)>ssid);
 
/* Collect the port status */
result = hfa384x_drvr_getconfig16(hw,
-   HFA384x_RID_PORTSTATUS,
-   );
+ 
HFA384x_RID_PORTSTATUS,
+ );
if (result) {
pr_debug
("getconfig(0x%02x) failed, result = %d\n",
@@ -1404,7 +1402,7 @@ void prism2sta_processing_defer(struct work_struct *data)
   ,
   HFA384x_RID_JOINREQUEST_LEN);
netdev_info(wlandev->netdev,
-  "linkstatus=ASSOCFAIL (re-submitting join)\n");
+   "linkstatus=ASSOCFAIL (re-submitting 
join)\n");
} else {

Re: [PATCH v6 1/3] clk: qcom: Configure the RCGs to a safe source as needed

[Stephen Boyd]

Quoting Amit Nischal (2018-05-03 04:57:37)
> On 2018-05-02 13:15, Stephen Boyd wrote:
> > Quoting Amit Nischal (2018-04-30 09:20:08)
> > 
> >> +}
> >> +
> >> +static void clk_rcg2_shared_disable(struct clk_hw *hw)
> >> +{
> >> +   struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> >> +   struct freq_tbl safe_src_tbl = { 0 };
> >> +
> >> +   /*
> >> +* Park the RCG at a safe configuration - sourced off from 
> >> safe source.
> >> +* Force enable and disable the RCG while configuring it to 
> >> safeguard
> >> +* against any update signal coming from the downstream clock.
> >> +* The current parent is still prepared and enabled at this 
> >> point, and
> >> +* the safe source is always on while application processor 
> >> subsystem
> >> +* is online. Therefore, the RCG can safely switch its parent.
> >> +*/
> >> +   safe_src_tbl.src = rcg->safe_src_index;
> >> +   clk_rcg2_shared_force_enable_clear(hw, _src_tbl);
> > 
> > This should then re-dirty the config register to have the software
> > frequency settings that existed in the hardware when disable was 
> > called.
> > Given that MND shouldn't be changed here, this should be as simple as
> > saving away the CFG register, forcing it to XO speed by changing the 
> > src
> > and disabling dual edge in the CFG register (please don't call
> > force_enable_clear with some frequency pointer, just do this inline
> > here), and then rewriting the cfg register with the "real" settings for
> > whatever frequency it's supposed to run at and then returning from this
> > function.
> > 
> > I guess we have to do a read cfg from hardware, write cfg, hit update
> > config, and then write cfg again each time we disable. For enable, we
> > just do an update config (if it's dirty?) inside of a force
> > enable/disable pair. And set_rate doesn't really change except it 
> > either
> > does or doesn't hit the update config bit if the clk is enabled or
> > disabled respectively.
> > 
> 
> We have done the below changes suggested by you and that logic seems to 
> be
> working fine. But we have one concern about leaving the RCG registers in
> dirty state and would like to have a little bit modification as 
> explained
> below:
> 
> Suggested Logic:
> 1. set_rate()--> Update CFG, M, N and D registers and don't hit the 
> update
>   bit if clock is disabled - call new 
> __clk_rcg2_configure().
>   Above will make the CFG register as dirty.
> 
> 2. _disable()--> 2.1 - Store the CFG register configuration in a 
> variable.
>   2.2 - Move to the safe source (XO) and hit the update 
> bit.
> It will only touch the CFG register and M, N, D
> register values will remain as it was.
>   2.3 - Write back the stored CFG value done in step #2.1
> This will again redirty the CFG register.
> 
> 3. _enable()--> Just hit the update bit as the configuration write will
>  be taken care in the steps #1 and #2.
> 
> It would be great if we don't redirty the CFG register and leave the RCG
> CFG register to at safe source(XO) in disable() path.
> 
> This would help us to debug the issues where device crashes and we want
> to dump the RCG registers to know whether from software, we have 
> actually
> moved to safe source or not. Otherwise, we would get the dirty register
> values in crash dumps.
> 
> So instead of writing back the stored CFG(corresponding to real rate
> settings) in disable path, we want to restore the stored CFG in enable
> path and then hit the update bit.
> CFG configuration store can happen at two places - set_rate() and 
> disable()
> path and above logic will be modified as below:
> 
> 1. set_rate()--> 1.1 - Update CFG, M, N and D registers and don't hit 
> the
> update bit if clock is disabled.
>   1.2 - Store CFG register value in 'current_cfg' member
> of 'rcg2' structure.
> 
> 2. _disable()--> 2.1 - Store the CFG register value in 'current_cfg' 
> before
> switching to the safe source (XO).
>   2.2 - Move to the safe source (XO) and hit the update 
> bit.
> Now RCG configuration wil not be dirty.
> 
> 3. _enable()--> 3.1 - Check for 'current_cfg' value and if 0 then 
> return.
>This would catch the below one time condition:
>- when clk_enable() gets call without set_rate().

We want clk_enable() to work without set_rate() though. So returning 0
if the value is 0 is wrong.

>  3.2 - Write the CFG value from 'current_cfg' to CFG 
> register.
>  3.2 - Hit the update bit as we have already written the 
> latest
>configuration in step #3.2.
>  3.3 - Clear the 'current_cfg' value.
> 
> We feel above logic will work as 

Re: [PATCH v6 1/3] clk: qcom: Configure the RCGs to a safe source as needed

[Stephen Boyd]

Quoting Amit Nischal (2018-05-03 04:57:37)
> On 2018-05-02 13:15, Stephen Boyd wrote:
> > Quoting Amit Nischal (2018-04-30 09:20:08)
> > 
> >> +}
> >> +
> >> +static void clk_rcg2_shared_disable(struct clk_hw *hw)
> >> +{
> >> +   struct clk_rcg2 *rcg = to_clk_rcg2(hw);
> >> +   struct freq_tbl safe_src_tbl = { 0 };
> >> +
> >> +   /*
> >> +* Park the RCG at a safe configuration - sourced off from 
> >> safe source.
> >> +* Force enable and disable the RCG while configuring it to 
> >> safeguard
> >> +* against any update signal coming from the downstream clock.
> >> +* The current parent is still prepared and enabled at this 
> >> point, and
> >> +* the safe source is always on while application processor 
> >> subsystem
> >> +* is online. Therefore, the RCG can safely switch its parent.
> >> +*/
> >> +   safe_src_tbl.src = rcg->safe_src_index;
> >> +   clk_rcg2_shared_force_enable_clear(hw, _src_tbl);
> > 
> > This should then re-dirty the config register to have the software
> > frequency settings that existed in the hardware when disable was 
> > called.
> > Given that MND shouldn't be changed here, this should be as simple as
> > saving away the CFG register, forcing it to XO speed by changing the 
> > src
> > and disabling dual edge in the CFG register (please don't call
> > force_enable_clear with some frequency pointer, just do this inline
> > here), and then rewriting the cfg register with the "real" settings for
> > whatever frequency it's supposed to run at and then returning from this
> > function.
> > 
> > I guess we have to do a read cfg from hardware, write cfg, hit update
> > config, and then write cfg again each time we disable. For enable, we
> > just do an update config (if it's dirty?) inside of a force
> > enable/disable pair. And set_rate doesn't really change except it 
> > either
> > does or doesn't hit the update config bit if the clk is enabled or
> > disabled respectively.
> > 
> 
> We have done the below changes suggested by you and that logic seems to 
> be
> working fine. But we have one concern about leaving the RCG registers in
> dirty state and would like to have a little bit modification as 
> explained
> below:
> 
> Suggested Logic:
> 1. set_rate()--> Update CFG, M, N and D registers and don't hit the 
> update
>   bit if clock is disabled - call new 
> __clk_rcg2_configure().
>   Above will make the CFG register as dirty.
> 
> 2. _disable()--> 2.1 - Store the CFG register configuration in a 
> variable.
>   2.2 - Move to the safe source (XO) and hit the update 
> bit.
> It will only touch the CFG register and M, N, D
> register values will remain as it was.
>   2.3 - Write back the stored CFG value done in step #2.1
> This will again redirty the CFG register.
> 
> 3. _enable()--> Just hit the update bit as the configuration write will
>  be taken care in the steps #1 and #2.
> 
> It would be great if we don't redirty the CFG register and leave the RCG
> CFG register to at safe source(XO) in disable() path.
> 
> This would help us to debug the issues where device crashes and we want
> to dump the RCG registers to know whether from software, we have 
> actually
> moved to safe source or not. Otherwise, we would get the dirty register
> values in crash dumps.
> 
> So instead of writing back the stored CFG(corresponding to real rate
> settings) in disable path, we want to restore the stored CFG in enable
> path and then hit the update bit.
> CFG configuration store can happen at two places - set_rate() and 
> disable()
> path and above logic will be modified as below:
> 
> 1. set_rate()--> 1.1 - Update CFG, M, N and D registers and don't hit 
> the
> update bit if clock is disabled.
>   1.2 - Store CFG register value in 'current_cfg' member
> of 'rcg2' structure.
> 
> 2. _disable()--> 2.1 - Store the CFG register value in 'current_cfg' 
> before
> switching to the safe source (XO).
>   2.2 - Move to the safe source (XO) and hit the update 
> bit.
> Now RCG configuration wil not be dirty.
> 
> 3. _enable()--> 3.1 - Check for 'current_cfg' value and if 0 then 
> return.
>This would catch the below one time condition:
>- when clk_enable() gets call without set_rate().

We want clk_enable() to work without set_rate() though. So returning 0
if the value is 0 is wrong.

>  3.2 - Write the CFG value from 'current_cfg' to CFG 
> register.
>  3.2 - Hit the update bit as we have already written the 
> latest
>configuration in step #3.2.
>  3.3 - Clear the 'current_cfg' value.
> 
> We feel above logic will work as 

Re: [PATCH v6 3/3] clk: qcom: Add Global Clock controller (GCC) driver for SDM845

[Stephen Boyd]

Quoting Amit Nischal (2018-05-04 03:45:12)
> On 2018-05-02 12:53, Stephen Boyd wrote:
> > Quoting Amit Nischal (2018-04-30 09:20:10)
> >> +
> >> +static struct clk_branch gcc_disp_gpll0_clk_src = {
> >> +   .halt_reg = 0x52004,
> >> +   .halt_check = BRANCH_HALT_DELAY,
> > 
> > What about this one? It's not a phy so I'm confused again why we're
> > unable to check the halt bit. To be clear(er), I don't see why we ever
> > want to have HALT_DELAY used. Hopefully we can remove that flag.
> > 
> > From what I recall, the flag is there for clks that don't toggle their
> > status bit at all, but that we know take a few cycles to ungate the
> > upstream clk. So we threw a delay into the code to make sure that when
> > clk_enable() returned, a driver wouldn't try to use hardware before the
> > clk was actually on. But these cases should pretty much never happen,
> > hence all the pushback against this flag.
> > 
> 
> For these "*gpll0_clk_src" and "*gpll0_div_clk" clocks, there is no halt
> bit to check the status and it is required to have delay for few cycles
> so that clock gets turned on before a client driver to use the hardware.

Ok.. but then why is there a 'halt_reg' configured for the clk?

> >> +
> >> +static struct clk_branch gcc_ufs_card_rx_symbol_0_clk = {
> >> +   .halt_reg = 0x75018,
> >> +   .halt_check = BRANCH_HALT_DELAY,
> > 
> > There are still HALT_DELAY flags for UFS though? Why?
> 
> For ufs_card tx/rx symbol clocks, we don't poll the status bit as
> per the recommendation from the HW team. We can change the halt_check
> type to newly implemented flag "BRANCH_HALT_SKIP". Please update us with
> your thoughts to change the flag to "BRANCH_HALT_SKIP".

Yes use HALT_SKIP please.

> 
> > 
> > Also, are you going to send DFS support for the QUP clks? I would like
> > to see that code merged soon.
> 
> Taniya has sent the patches for DFS support for QUP clocks.
> https://patchwork.kernel.org/patch/10376951/
> 

I'll take a look.

Re: [PATCH v6 3/3] clk: qcom: Add Global Clock controller (GCC) driver for SDM845

[Stephen Boyd]

Quoting Amit Nischal (2018-05-04 03:45:12)
> On 2018-05-02 12:53, Stephen Boyd wrote:
> > Quoting Amit Nischal (2018-04-30 09:20:10)
> >> +
> >> +static struct clk_branch gcc_disp_gpll0_clk_src = {
> >> +   .halt_reg = 0x52004,
> >> +   .halt_check = BRANCH_HALT_DELAY,
> > 
> > What about this one? It's not a phy so I'm confused again why we're
> > unable to check the halt bit. To be clear(er), I don't see why we ever
> > want to have HALT_DELAY used. Hopefully we can remove that flag.
> > 
> > From what I recall, the flag is there for clks that don't toggle their
> > status bit at all, but that we know take a few cycles to ungate the
> > upstream clk. So we threw a delay into the code to make sure that when
> > clk_enable() returned, a driver wouldn't try to use hardware before the
> > clk was actually on. But these cases should pretty much never happen,
> > hence all the pushback against this flag.
> > 
> 
> For these "*gpll0_clk_src" and "*gpll0_div_clk" clocks, there is no halt
> bit to check the status and it is required to have delay for few cycles
> so that clock gets turned on before a client driver to use the hardware.

Ok.. but then why is there a 'halt_reg' configured for the clk?

> >> +
> >> +static struct clk_branch gcc_ufs_card_rx_symbol_0_clk = {
> >> +   .halt_reg = 0x75018,
> >> +   .halt_check = BRANCH_HALT_DELAY,
> > 
> > There are still HALT_DELAY flags for UFS though? Why?
> 
> For ufs_card tx/rx symbol clocks, we don't poll the status bit as
> per the recommendation from the HW team. We can change the halt_check
> type to newly implemented flag "BRANCH_HALT_SKIP". Please update us with
> your thoughts to change the flag to "BRANCH_HALT_SKIP".

Yes use HALT_SKIP please.

> 
> > 
> > Also, are you going to send DFS support for the QUP clks? I would like
> > to see that code merged soon.
> 
> Taniya has sent the patches for DFS support for QUP clocks.
> https://patchwork.kernel.org/patch/10376951/
> 

I'll take a look.

Re: [PATCH v3 1/2] clk: x86: Add ST oscout platform clock

[Stephen Boyd]

Quoting Akshu Agrawal (2018-05-04 10:07:18)
> Stoney SoC provides oscout clock. This clock can support 25Mhz and
> 48Mhz of frequency.
> The clock is available for general system use.
> 
> Signed-off-by: Akshu Agrawal 
> ---
> v2: config change, added SPDX tag and used clk_hw_register_.
> v3: Fix kbuild warning for checking of NULL pointer

I replied on some older patch. Same comments apply still.

Re: [PATCH v3 1/2] clk: x86: Add ST oscout platform clock

[Stephen Boyd]

Quoting Akshu Agrawal (2018-05-04 10:07:18)
> Stoney SoC provides oscout clock. This clock can support 25Mhz and
> 48Mhz of frequency.
> The clock is available for general system use.
> 
> Signed-off-by: Akshu Agrawal 
> ---
> v2: config change, added SPDX tag and used clk_hw_register_.
> v3: Fix kbuild warning for checking of NULL pointer

I replied on some older patch. Same comments apply still.

Re: [PATCH V3 1/2] clk: imx6sx: add missing lvds2 clock to the clock tree

[Stephen Boyd]

Quoting Anson Huang (2018-04-20 00:38:10)
> i.MX6SX has lvds2 (analog clock2), an I/O clock like lvds1.
> And this lvds2, along with lvds1, can be used to provide
> external clock source to the internal pll, such as pll4_audio
> and pll5_video.
> 
> This patch mainly adds the lvds2 to the clock tree and fix its
> relationship with pll accordingly.
> 
> Signed-off-by: Anson Huang 
> Signed-off-by: Shengjiu Wang 
> Reviewed-by: Rob Herring 
> ---

Applied to clk-next

Re: [PATCH V3 1/2] clk: imx6sx: add missing lvds2 clock to the clock tree

[Stephen Boyd]

Quoting Anson Huang (2018-04-20 00:38:10)
> i.MX6SX has lvds2 (analog clock2), an I/O clock like lvds1.
> And this lvds2, along with lvds1, can be used to provide
> external clock source to the internal pll, such as pll4_audio
> and pll5_video.
> 
> This patch mainly adds the lvds2 to the clock tree and fix its
> relationship with pll accordingly.
> 
> Signed-off-by: Anson Huang 
> Signed-off-by: Shengjiu Wang 
> Reviewed-by: Rob Herring 
> ---

Applied to clk-next

Re: [PATCH 6/9] firmware: print firmware name on fallback path

[Andres Rodriguez]

On 2018-05-03 07:42 PM, Luis R. Rodriguez wrote:

On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:

Previously, one could assume the firmware name from the preceding
message: "Direct firmware load for {name} failed with error %d".

However, with the new firmware_request_nowarn() entrypoint, the message
outlined above will not always be printed.


I though the whole point was to not print an error message. What if
we want later to disable this error message? This would prove a bit
pointless.

Let's discuss the exact semantics desired here. Why would only the
fallback be desirable here?

Andres, Kalle?

After we address this I'll address resubmitting this lat patch
along with the last one. For now I'll skip it.


You are correct. I initially thought it would be useful to know that the 
usermode fallback was being triggered. And for that message to be useful 
we would need a fw name.


But now that you point it out, this behaviour is inconsistent with the 
_nowarn() definition. We shouldn't have a message in the first place.


So it might be better to instead have:

if (!(opt_flags & FW_OPT_NO_WARN) )
dev_warn(device, "Falling back to user helper\n");

No need to add the firmware name, cause we either:
a) FW_OPT_NO_WARN is set and no messages are printed, or
b) FW_OPT_NO_WARN is not set and we get both messages.

Yay, nay?

Regards,
Andres



   Luis


Therefore, we add the firmware name to the fallback path spew in order
to associate it with the appropriate request.

Signed-off-by: Andres Rodriguez 
---
  drivers/base/firmware_loader/fallback.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/firmware_loader/fallback.c 
b/drivers/base/firmware_loader/fallback.c
index e75928458489..1a47ddc70c31 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -669,6 +669,6 @@ int fw_sysfs_fallback(struct firmware *fw, const char *name,
if (!fw_run_sysfs_fallback(opt_flags))
return ret;
  
-	dev_warn(device, "Falling back to user helper\n");

+   dev_warn(device, "Falling back to user helper for %s\n", name);
return fw_load_from_user_helper(fw, name, device, opt_flags);
  }
--
2.14.1

Re: [PATCH 6/9] firmware: print firmware name on fallback path

[Andres Rodriguez]

On 2018-05-03 07:42 PM, Luis R. Rodriguez wrote:

On Mon, Apr 23, 2018 at 04:12:02PM -0400, Andres Rodriguez wrote:

Previously, one could assume the firmware name from the preceding
message: "Direct firmware load for {name} failed with error %d".

However, with the new firmware_request_nowarn() entrypoint, the message
outlined above will not always be printed.


I though the whole point was to not print an error message. What if
we want later to disable this error message? This would prove a bit
pointless.

Let's discuss the exact semantics desired here. Why would only the
fallback be desirable here?

Andres, Kalle?

After we address this I'll address resubmitting this lat patch
along with the last one. For now I'll skip it.


You are correct. I initially thought it would be useful to know that the 
usermode fallback was being triggered. And for that message to be useful 
we would need a fw name.


But now that you point it out, this behaviour is inconsistent with the 
_nowarn() definition. We shouldn't have a message in the first place.


So it might be better to instead have:

if (!(opt_flags & FW_OPT_NO_WARN) )
dev_warn(device, "Falling back to user helper\n");

No need to add the firmware name, cause we either:
a) FW_OPT_NO_WARN is set and no messages are printed, or
b) FW_OPT_NO_WARN is not set and we get both messages.

Yay, nay?

Regards,
Andres



   Luis


Therefore, we add the firmware name to the fallback path spew in order
to associate it with the appropriate request.

Signed-off-by: Andres Rodriguez 
---
  drivers/base/firmware_loader/fallback.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/firmware_loader/fallback.c 
b/drivers/base/firmware_loader/fallback.c
index e75928458489..1a47ddc70c31 100644
--- a/drivers/base/firmware_loader/fallback.c
+++ b/drivers/base/firmware_loader/fallback.c
@@ -669,6 +669,6 @@ int fw_sysfs_fallback(struct firmware *fw, const char *name,
if (!fw_run_sysfs_fallback(opt_flags))
return ret;
  
-	dev_warn(device, "Falling back to user helper\n");

+   dev_warn(device, "Falling back to user helper for %s\n", name);
return fw_load_from_user_helper(fw, name, device, opt_flags);
  }
--
2.14.1

Re: [PATCH] clk: imx6ul: fix periph clk2 clock mux selection

[Stephen Boyd]

Quoting Stefan Agner (2018-04-18 05:52:54)
> According to the data sheet the 3rd choice is the bypass clock
> of pll2. This should not have any effect in practice as this
> selection is not used currently.
> 
> Signed-off-by: Stefan Agner 
> ---

Applied to clk-next

Re: [PATCH] clk: imx6ul: fix periph clk2 clock mux selection

[Stephen Boyd]

Quoting Stefan Agner (2018-04-18 05:52:54)
> According to the data sheet the 3rd choice is the bypass clock
> of pll2. This should not have any effect in practice as this
> selection is not used currently.
> 
> Signed-off-by: Stefan Agner 
> ---

Applied to clk-next

Re: [PATCH v1] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

[Stephen Boyd]

Quoting Taniya Das (2018-05-03 02:09:57)
> Hello Stephen,
> 
> I have tested the below patch & didn't see any issues.

Alright. Thanks! Can I take that as a "Tested-by"?

Re: [PATCH v1] clk: qcom: gdsc: Add support to poll CFG register to check GDSC state

[Stephen Boyd]

Quoting Taniya Das (2018-05-03 02:09:57)
> Hello Stephen,
> 
> I have tested the below patch & didn't see any issues.

Alright. Thanks! Can I take that as a "Tested-by"?

Re: [RESEND PATCH] clk: stm32: fix: stm32 clock drivers are not compiled by default

[Stephen Boyd]

Quoting Alexandre Torgue (2018-05-04 00:54:16)
> Stephen
> 
> On 05/03/2018 08:40 AM, gabriel.fernan...@st.com wrote:
> > From: Gabriel Fernandez 
> > 
> > Clock driver is mandatory if the machine is selected.
> > Then don't use 'bool' and 'depends on' commands, but 'def_bool'
> > with the machine(s).
> > 
> > Fixes: da32d3539fca ("clk: stm32: add configuration flags for each of the 
> > stm32 drivers")
> > 
> 
> Sorry to insist but we need it to have STM32 MCUs booting on Kernel v4.17.

Thanks for the bump. I missed this one. Of course, the user can still select
the configs now, just it's annoying for upgrade path.

> 
> > Signed-off-by: Gabriel Fernandez 
> > Acked-by: Alexandre TORGUE 
> > ---
> >   drivers/clk/Kconfig | 6 ++
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 24a5bc3..721572a 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -266,15 +266,13 @@ config COMMON_CLK_STM32MP157
> > Support for stm32mp157 SoC family clocks
> >   
> >   config COMMON_CLK_STM32F
> > - bool "Clock driver for stm32f4 and stm32f7 SoC families"
> > - depends on MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746
> > + def_bool COMMON_CLK && (MACH_STM32F429 || MACH_STM32F469 || 
> > MACH_STM32F746)

But the point of the change this patch is fixing was to expose these to
the user to turn off they wanted. You'll need to do something like that
again here, instead of removing the prompt and replacing it with a
def_bool.

So this patch instead? It leaves it around for the whole arch, but
limits the default to be the machines that matter. I suppose we could
put an 'if EXPERT' on the bool part too if we don't even want to expose
the options to normal users.

-8<-
From: Gabriel Fernandez 
Date: Thu, 3 May 2018 08:40:09 +0200
Subject: [PATCH] clk: stm32: fix: stm32 clock drivers are not compiled by
 default
Cc: ,


Clock driver is mandatory if the machine is selected. Add a default
of the machines that need the clk driver, so that the user can turn it
off if they want, but otherwise it's exposed on the SoCs the driver
is for.

Fixes: da32d3539fca ("clk: stm32: add configuration flags for each of the stm32 
drivers")
Signed-off-by: Gabriel Fernandez 
Acked-by: Alexandre TORGUE 
Signed-off-by: Stephen Boyd 
---
 drivers/clk/Kconfig | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 41492e980ef4..ac3f0e2bc03f 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -267,15 +267,16 @@ config COMMON_CLK_STM32MP157
 
 config COMMON_CLK_STM32F
bool "Clock driver for stm32f4 and stm32f7 SoC families"
-   depends on MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746
+   depends on ARCH_STM32
+   default MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746
help
---help---
  Support for stm32f4 and stm32f7 SoC families clocks
 
 config COMMON_CLK_STM32H7
bool "Clock driver for stm32h7 SoC family"
-   depends on MACH_STM32H743
-   help
+   depends on ARCH_STM32
+   default MACH_STM32H743
---help---
  Support for stm32h7 SoC family clocks
 
-- 
Sent by a computer through tubes

Re: [RESEND PATCH] clk: stm32: fix: stm32 clock drivers are not compiled by default

[Stephen Boyd]

Quoting Alexandre Torgue (2018-05-04 00:54:16)
> Stephen
> 
> On 05/03/2018 08:40 AM, gabriel.fernan...@st.com wrote:
> > From: Gabriel Fernandez 
> > 
> > Clock driver is mandatory if the machine is selected.
> > Then don't use 'bool' and 'depends on' commands, but 'def_bool'
> > with the machine(s).
> > 
> > Fixes: da32d3539fca ("clk: stm32: add configuration flags for each of the 
> > stm32 drivers")
> > 
> 
> Sorry to insist but we need it to have STM32 MCUs booting on Kernel v4.17.

Thanks for the bump. I missed this one. Of course, the user can still select
the configs now, just it's annoying for upgrade path.

> 
> > Signed-off-by: Gabriel Fernandez 
> > Acked-by: Alexandre TORGUE 
> > ---
> >   drivers/clk/Kconfig | 6 ++
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
> > index 24a5bc3..721572a 100644
> > --- a/drivers/clk/Kconfig
> > +++ b/drivers/clk/Kconfig
> > @@ -266,15 +266,13 @@ config COMMON_CLK_STM32MP157
> > Support for stm32mp157 SoC family clocks
> >   
> >   config COMMON_CLK_STM32F
> > - bool "Clock driver for stm32f4 and stm32f7 SoC families"
> > - depends on MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746
> > + def_bool COMMON_CLK && (MACH_STM32F429 || MACH_STM32F469 || 
> > MACH_STM32F746)

But the point of the change this patch is fixing was to expose these to
the user to turn off they wanted. You'll need to do something like that
again here, instead of removing the prompt and replacing it with a
def_bool.

So this patch instead? It leaves it around for the whole arch, but
limits the default to be the machines that matter. I suppose we could
put an 'if EXPERT' on the bool part too if we don't even want to expose
the options to normal users.

-8<-
From: Gabriel Fernandez 
Date: Thu, 3 May 2018 08:40:09 +0200
Subject: [PATCH] clk: stm32: fix: stm32 clock drivers are not compiled by
 default
Cc: ,


Clock driver is mandatory if the machine is selected. Add a default
of the machines that need the clk driver, so that the user can turn it
off if they want, but otherwise it's exposed on the SoCs the driver
is for.

Fixes: da32d3539fca ("clk: stm32: add configuration flags for each of the stm32 
drivers")
Signed-off-by: Gabriel Fernandez 
Acked-by: Alexandre TORGUE 
Signed-off-by: Stephen Boyd 
---
 drivers/clk/Kconfig | 7 ---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig
index 41492e980ef4..ac3f0e2bc03f 100644
--- a/drivers/clk/Kconfig
+++ b/drivers/clk/Kconfig
@@ -267,15 +267,16 @@ config COMMON_CLK_STM32MP157
 
 config COMMON_CLK_STM32F
bool "Clock driver for stm32f4 and stm32f7 SoC families"
-   depends on MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746
+   depends on ARCH_STM32
+   default MACH_STM32F429 || MACH_STM32F469 || MACH_STM32F746
help
---help---
  Support for stm32f4 and stm32f7 SoC families clocks
 
 config COMMON_CLK_STM32H7
bool "Clock driver for stm32h7 SoC family"
-   depends on MACH_STM32H743
-   help
+   depends on ARCH_STM32
+   default MACH_STM32H743
---help---
  Support for stm32h7 SoC family clocks
 
-- 
Sent by a computer through tubes

Re: [PATCH v7 00/16] tracing: probeevent: Improve fetcharg features

[Masami Hiramatsu]

On Fri, 4 May 2018 12:06:42 -0400
Steven Rostedt  wrote:

> On Sat, 5 May 2018 00:48:28 +0900
> Masami Hiramatsu  wrote:
> 
> > So the syntax will be
> > 
> > p[:EVENT] SYM[(CAST)|+OFFS] [FETCHARG]
> > 
> > And here is an example;
> > 
> > p:myevent vfs_read(void *file, char *buf, size_t count, void *pos) $arg1 
> > $arg2
> 
> If we do this, why bother with $arg1 $arg2?

User may want to trace only some of them. :)

> 
> We could allow this to be an alternative format?

I think we can skip passing $args, which implies trace all arguments.

p[:EVENT] SYM[(CAST)|+OFFS] [FETCHARG(*)]

*) if SYM(CAST) is given but no FETCHARG, which implies to trace
all arguments in the CAST.

> > In this case inside '()' will be analyzed and packed as something
> > like "reference type" data and it is used when converting "$argN".
> > And maybe we can provide $args special variable to record all
> > arguments (it can be available only when the (CAST) is given).
> > 
> > This gives the user a consistent model; if you just give a symbol
> > the arguments may not be correctly translated. but if you give a
> > type-casting information, it will be much better.
> > 
> > > 
> > > Also, when looking at the kprobe code, I was looking at this function:
> > >   
> > > > /* Ftrace callback handler for kprobes -- called under preepmt disabed 
> > > > */
> > > > void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> > > >struct ftrace_ops *ops, struct pt_regs *regs)
> > > > {
> > > > struct kprobe *p;
> > > > struct kprobe_ctlblk *kcb;
> > > > 
> > > > /* Preempt is disabled by ftrace */
> > > > p = get_kprobe((kprobe_opcode_t *)ip);
> > > > if (unlikely(!p) || kprobe_disabled(p))
> > > > return;
> > > > 
> > > > kcb = get_kprobe_ctlblk();
> > > > if (kprobe_running()) {
> > > > kprobes_inc_nmissed_count(p);
> > > > } else {
> > > > unsigned long orig_ip = regs->ip;
> > > > /* Kprobe handler expects regs->ip = ip + 1 as 
> > > > breakpoint hit */
> > > > regs->ip = ip + sizeof(kprobe_opcode_t);
> > > > 
> > > > /* To emulate trap based kprobes, preempt_disable here 
> > > > */
> > > > preempt_disable();
> > > > __this_cpu_write(current_kprobe, p);
> > > > kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> > > > if (!p->pre_handler || !p->pre_handler(p, regs)) {
> > > > __skip_singlestep(p, regs, kcb, orig_ip);
> > > > preempt_enable_no_resched();  
> > > 
> > > This preemption disabling and enabling looks rather strange. Looking at
> > > git blame, it appears this was added for jprobes. Can we remove it now
> > > that jprobes is going away?  
> > 
> > No, that is not for jprobes but for compatibility with kprobe's user
> > handler. Since this transformation is done silently, user can not
> > change their handler for ftrace case. So we need to keep this condition
> > same as original kprobes.
> > 
> > And anyway, for using smp_processor_id() for accessing per-cpu,
> > we should disable preemption, correct?
> 
> But as stated at the start of the function:
> 
>  /* Preempt is disabled by ftrace */

Ah, yes. So this is only for the jprobes.

> 
> 
> The reason I ask, is that we have for this function:
> 
>   /* To emulate trap based kprobes, preempt_disable here */
>   preempt_disable();
>   __this_cpu_write(current_kprobe, p);
>   kcb->kprobe_status = KPROBE_HIT_ACTIVE;
>   if (!p->pre_handler || !p->pre_handler(p, regs)) {
>   __skip_singlestep(p, regs, kcb, orig_ip);
>   preempt_enable_no_resched();
>   }
> 
> And in arch/x86/kernel/kprobes/core.c we have:
> 
>   preempt_disable();
> 
>   kcb = get_kprobe_ctlblk();
>   p = get_kprobe(addr);
> 
>   if (p) {
>   if (kprobe_running()) {
>   if (reenter_kprobe(p, regs, kcb))
>   return 1;
>   } else {
>   set_current_kprobe(p, regs, kcb);
>   kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> 
>   /*
>* If we have no pre-handler or it returned 0, we
>* continue with normal processing.  If we have a
>* pre-handler and it returned non-zero, it prepped
>* for calling the break_handler below on re-entry
>* for jprobe processing, so get out doing nothing
>* more here.
>*/
>   if (!p->pre_handler || !p->pre_handler(p, regs))
>   setup_singlestep(p, regs, kcb, 0);
>   return 1;
> 

Re: [PATCH v7 00/16] tracing: probeevent: Improve fetcharg features

[Masami Hiramatsu]

On Fri, 4 May 2018 12:06:42 -0400
Steven Rostedt  wrote:

> On Sat, 5 May 2018 00:48:28 +0900
> Masami Hiramatsu  wrote:
> 
> > So the syntax will be
> > 
> > p[:EVENT] SYM[(CAST)|+OFFS] [FETCHARG]
> > 
> > And here is an example;
> > 
> > p:myevent vfs_read(void *file, char *buf, size_t count, void *pos) $arg1 
> > $arg2
> 
> If we do this, why bother with $arg1 $arg2?

User may want to trace only some of them. :)

> 
> We could allow this to be an alternative format?

I think we can skip passing $args, which implies trace all arguments.

p[:EVENT] SYM[(CAST)|+OFFS] [FETCHARG(*)]

*) if SYM(CAST) is given but no FETCHARG, which implies to trace
all arguments in the CAST.

> > In this case inside '()' will be analyzed and packed as something
> > like "reference type" data and it is used when converting "$argN".
> > And maybe we can provide $args special variable to record all
> > arguments (it can be available only when the (CAST) is given).
> > 
> > This gives the user a consistent model; if you just give a symbol
> > the arguments may not be correctly translated. but if you give a
> > type-casting information, it will be much better.
> > 
> > > 
> > > Also, when looking at the kprobe code, I was looking at this function:
> > >   
> > > > /* Ftrace callback handler for kprobes -- called under preepmt disabed 
> > > > */
> > > > void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip,
> > > >struct ftrace_ops *ops, struct pt_regs *regs)
> > > > {
> > > > struct kprobe *p;
> > > > struct kprobe_ctlblk *kcb;
> > > > 
> > > > /* Preempt is disabled by ftrace */
> > > > p = get_kprobe((kprobe_opcode_t *)ip);
> > > > if (unlikely(!p) || kprobe_disabled(p))
> > > > return;
> > > > 
> > > > kcb = get_kprobe_ctlblk();
> > > > if (kprobe_running()) {
> > > > kprobes_inc_nmissed_count(p);
> > > > } else {
> > > > unsigned long orig_ip = regs->ip;
> > > > /* Kprobe handler expects regs->ip = ip + 1 as 
> > > > breakpoint hit */
> > > > regs->ip = ip + sizeof(kprobe_opcode_t);
> > > > 
> > > > /* To emulate trap based kprobes, preempt_disable here 
> > > > */
> > > > preempt_disable();
> > > > __this_cpu_write(current_kprobe, p);
> > > > kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> > > > if (!p->pre_handler || !p->pre_handler(p, regs)) {
> > > > __skip_singlestep(p, regs, kcb, orig_ip);
> > > > preempt_enable_no_resched();  
> > > 
> > > This preemption disabling and enabling looks rather strange. Looking at
> > > git blame, it appears this was added for jprobes. Can we remove it now
> > > that jprobes is going away?  
> > 
> > No, that is not for jprobes but for compatibility with kprobe's user
> > handler. Since this transformation is done silently, user can not
> > change their handler for ftrace case. So we need to keep this condition
> > same as original kprobes.
> > 
> > And anyway, for using smp_processor_id() for accessing per-cpu,
> > we should disable preemption, correct?
> 
> But as stated at the start of the function:
> 
>  /* Preempt is disabled by ftrace */

Ah, yes. So this is only for the jprobes.

> 
> 
> The reason I ask, is that we have for this function:
> 
>   /* To emulate trap based kprobes, preempt_disable here */
>   preempt_disable();
>   __this_cpu_write(current_kprobe, p);
>   kcb->kprobe_status = KPROBE_HIT_ACTIVE;
>   if (!p->pre_handler || !p->pre_handler(p, regs)) {
>   __skip_singlestep(p, regs, kcb, orig_ip);
>   preempt_enable_no_resched();
>   }
> 
> And in arch/x86/kernel/kprobes/core.c we have:
> 
>   preempt_disable();
> 
>   kcb = get_kprobe_ctlblk();
>   p = get_kprobe(addr);
> 
>   if (p) {
>   if (kprobe_running()) {
>   if (reenter_kprobe(p, regs, kcb))
>   return 1;
>   } else {
>   set_current_kprobe(p, regs, kcb);
>   kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> 
>   /*
>* If we have no pre-handler or it returned 0, we
>* continue with normal processing.  If we have a
>* pre-handler and it returned non-zero, it prepped
>* for calling the break_handler below on re-entry
>* for jprobe processing, so get out doing nothing
>* more here.
>*/
>   if (!p->pre_handler || !p->pre_handler(p, regs))
>   setup_singlestep(p, regs, kcb, 0);
>   return 1;
> 
> 
> Which is why I thought it was for 

Re: [v2 PATCH 1/1] tg3: fix meaningless hw_stats reading after tg3_halt memset 0 hw_stats

[Zumeng Chen]

On 05/03/2018 01:04 PM, Michael Chan wrote:

On Wed, May 2, 2018 at 5:30 PM, Zumeng Chen  wrote:

On 2018年05月03日 01:32, Michael Chan wrote:

On Wed, May 2, 2018 at 3:27 AM, Zumeng Chen  wrote:

On 2018年05月02日 13:12, Michael Chan wrote:

On Tue, May 1, 2018 at 5:42 PM, Zumeng Chen 
wrote:


diff --git a/drivers/net/ethernet/broadcom/tg3.h
b/drivers/net/ethernet/broadcom/tg3.h
index 3b5e98e..c61d83c 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -3102,6 +3102,7 @@ enum TG3_FLAGS {
   TG3_FLAG_ROBOSWITCH,
   TG3_FLAG_ONE_DMA_AT_ONCE,
   TG3_FLAG_RGMII_MODE,
+   TG3_FLAG_HALT,

I think you should be able to use the existing INIT_COMPLETE flag


No,  it will bring the uncertain factors into the existed complicate
logic
of INIT_COMPLETE.
And I think it's very simple logic here to fix the meaningless hw_stats
reading and the problem
of commit f5992b72. I even suspect if you have read INIT_COMPLETE related
codes carefully.


We should use an existing flag whenever appropriate


I disagree. This is sort of blahblah...

I don't want to see another flag added that is practically the same as
!INIT_COMPLETE.  The driver already has close to one hundred flags.
Adding a new flag that is similar to an existing flag will just make
the code more difficult to understand and maintain.

If you don't want to fix it the cleaner way, Siva or I will fix it.


Feel free to go, I just take a double look, INIT_COMPLETE can directly 
be used as follows:


diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c

index 08bbb63..0e04fd7 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8733,14 +8733,11 @@ static void tg3_free_consistent(struct tg3 *tp)
tg3_mem_rx_release(tp);
tg3_mem_tx_release(tp);

-   /* Protect tg3_get_stats64() from reading freed tp->hw_stats. */
-   tg3_full_lock(tp, 0);
if (tp->hw_stats) {
dma_free_coherent(>pdev->dev, sizeof(struct 
tg3_hw_stats),

  tp->hw_stats, tp->stats_mapping);
tp->hw_stats = NULL;
}
-   tg3_full_unlock(tp);
 }

 /*
@@ -14178,7 +14175,7 @@ static void tg3_get_stats64(struct net_device *dev,
struct tg3 *tp = netdev_priv(dev);

spin_lock_bh(>lock);
-   if (!tp->hw_stats) {
+   if (!tp->hw_stats || tg3_flag(tp, INIT_COMPLETE)) {
*stats = tp->net_stats_prev;
spin_unlock_bh(>lock);
return;

Cheers,
Zumeng

Re: [v2 PATCH 1/1] tg3: fix meaningless hw_stats reading after tg3_halt memset 0 hw_stats

[Zumeng Chen]

On 05/03/2018 01:04 PM, Michael Chan wrote:

On Wed, May 2, 2018 at 5:30 PM, Zumeng Chen  wrote:

On 2018年05月03日 01:32, Michael Chan wrote:

On Wed, May 2, 2018 at 3:27 AM, Zumeng Chen  wrote:

On 2018年05月02日 13:12, Michael Chan wrote:

On Tue, May 1, 2018 at 5:42 PM, Zumeng Chen 
wrote:


diff --git a/drivers/net/ethernet/broadcom/tg3.h
b/drivers/net/ethernet/broadcom/tg3.h
index 3b5e98e..c61d83c 100644
--- a/drivers/net/ethernet/broadcom/tg3.h
+++ b/drivers/net/ethernet/broadcom/tg3.h
@@ -3102,6 +3102,7 @@ enum TG3_FLAGS {
   TG3_FLAG_ROBOSWITCH,
   TG3_FLAG_ONE_DMA_AT_ONCE,
   TG3_FLAG_RGMII_MODE,
+   TG3_FLAG_HALT,

I think you should be able to use the existing INIT_COMPLETE flag


No,  it will bring the uncertain factors into the existed complicate
logic
of INIT_COMPLETE.
And I think it's very simple logic here to fix the meaningless hw_stats
reading and the problem
of commit f5992b72. I even suspect if you have read INIT_COMPLETE related
codes carefully.


We should use an existing flag whenever appropriate


I disagree. This is sort of blahblah...

I don't want to see another flag added that is practically the same as
!INIT_COMPLETE.  The driver already has close to one hundred flags.
Adding a new flag that is similar to an existing flag will just make
the code more difficult to understand and maintain.

If you don't want to fix it the cleaner way, Siva or I will fix it.


Feel free to go, I just take a double look, INIT_COMPLETE can directly 
be used as follows:


diff --git a/drivers/net/ethernet/broadcom/tg3.c 
b/drivers/net/ethernet/broadcom/tg3.c

index 08bbb63..0e04fd7 100644
--- a/drivers/net/ethernet/broadcom/tg3.c
+++ b/drivers/net/ethernet/broadcom/tg3.c
@@ -8733,14 +8733,11 @@ static void tg3_free_consistent(struct tg3 *tp)
tg3_mem_rx_release(tp);
tg3_mem_tx_release(tp);

-   /* Protect tg3_get_stats64() from reading freed tp->hw_stats. */
-   tg3_full_lock(tp, 0);
if (tp->hw_stats) {
dma_free_coherent(>pdev->dev, sizeof(struct 
tg3_hw_stats),

  tp->hw_stats, tp->stats_mapping);
tp->hw_stats = NULL;
}
-   tg3_full_unlock(tp);
 }

 /*
@@ -14178,7 +14175,7 @@ static void tg3_get_stats64(struct net_device *dev,
struct tg3 *tp = netdev_priv(dev);

spin_lock_bh(>lock);
-   if (!tp->hw_stats) {
+   if (!tp->hw_stats || tg3_flag(tp, INIT_COMPLETE)) {
*stats = tp->net_stats_prev;
spin_unlock_bh(>lock);
return;

Cheers,
Zumeng

Re: [PATCH 1/2] tpm: do not suspend/resume if power stays on

[Greg KH]

On Wed, May 02, 2018 at 05:38:28PM +0300, Jarkko Sakkinen wrote:
> From: Enric Balletbo i Serra 
> 
> commit b5d0ebc99bf5d0801a5ecbe958caa3d68b8eaee8 upstream
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao 
> Signed-off-by: Enric Balletbo i Serra 
> Reviewed-by: Jason Gunthorpe 
> Reviewed-by: Jarkko Sakkinen 
> Signed-off-by: Jarkko Sakkinen 
> Signed-off-by: James Morris 
> ---
>  drivers/char/tpm/tpm-interface.c | 3 +++
>  drivers/char/tpm/tpm.h   | 2 ++
>  drivers/char/tpm/tpm_of.c| 3 +++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index 830d7e30e508..5463b649bdf1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -969,6 +969,9 @@ int tpm_pm_suspend(struct device *dev)
>   if (chip == NULL)
>   return -ENODEV;
>  
> + if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
> + return 0;
> +
>   if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>   tpm2_shutdown(chip, TPM2_SU_STATE);
>   return 0;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index aa4299cf7e5a..41756a9e9ad8 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -143,6 +143,8 @@ enum tpm_chip_flags {
>   TPM_CHIP_FLAG_TPM2  = BIT(1),
>   TPM_CHIP_FLAG_IRQ   = BIT(2),
>   TPM_CHIP_FLAG_VIRTUAL   = BIT(3),
> + TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
> + TPM_CHIP_FLAG_ALWAYS_POWERED= BIT(5),
>  };
>  
>  struct tpm_chip {
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 570f30c5c5f4..669f4a046398 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -37,6 +37,9 @@ int read_log(struct tpm_bios_log *log)
>   return -ENODEV;
>   }
>  
> + if (of_property_read_bool(np, "powered-while-suspended"))
> + chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
> +

This last line here blows up the build, there is no chip variable
defined in this function :(

So I have to drop both of these patches, from both 4.4.y and 4.9.y
queues right now.  Can you fix this up and resend them?

thanks,

greg k-h

Re: [PATCH 1/2] tpm: do not suspend/resume if power stays on

[Greg KH]

On Wed, May 02, 2018 at 05:38:28PM +0300, Jarkko Sakkinen wrote:
> From: Enric Balletbo i Serra 
> 
> commit b5d0ebc99bf5d0801a5ecbe958caa3d68b8eaee8 upstream
> 
> The suspend/resume behavior of the TPM can be controlled by setting
> "powered-while-suspended" in the DTS. This is useful for the cases
> when hardware does not power-off the TPM.
> 
> Signed-off-by: Sonny Rao 
> Signed-off-by: Enric Balletbo i Serra 
> Reviewed-by: Jason Gunthorpe 
> Reviewed-by: Jarkko Sakkinen 
> Signed-off-by: Jarkko Sakkinen 
> Signed-off-by: James Morris 
> ---
>  drivers/char/tpm/tpm-interface.c | 3 +++
>  drivers/char/tpm/tpm.h   | 2 ++
>  drivers/char/tpm/tpm_of.c| 3 +++
>  3 files changed, 8 insertions(+)
> 
> diff --git a/drivers/char/tpm/tpm-interface.c 
> b/drivers/char/tpm/tpm-interface.c
> index 830d7e30e508..5463b649bdf1 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -969,6 +969,9 @@ int tpm_pm_suspend(struct device *dev)
>   if (chip == NULL)
>   return -ENODEV;
>  
> + if (chip->flags & TPM_CHIP_FLAG_ALWAYS_POWERED)
> + return 0;
> +
>   if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>   tpm2_shutdown(chip, TPM2_SU_STATE);
>   return 0;
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index aa4299cf7e5a..41756a9e9ad8 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -143,6 +143,8 @@ enum tpm_chip_flags {
>   TPM_CHIP_FLAG_TPM2  = BIT(1),
>   TPM_CHIP_FLAG_IRQ   = BIT(2),
>   TPM_CHIP_FLAG_VIRTUAL   = BIT(3),
> + TPM_CHIP_FLAG_HAVE_TIMEOUTS = BIT(4),
> + TPM_CHIP_FLAG_ALWAYS_POWERED= BIT(5),
>  };
>  
>  struct tpm_chip {
> diff --git a/drivers/char/tpm/tpm_of.c b/drivers/char/tpm/tpm_of.c
> index 570f30c5c5f4..669f4a046398 100644
> --- a/drivers/char/tpm/tpm_of.c
> +++ b/drivers/char/tpm/tpm_of.c
> @@ -37,6 +37,9 @@ int read_log(struct tpm_bios_log *log)
>   return -ENODEV;
>   }
>  
> + if (of_property_read_bool(np, "powered-while-suspended"))
> + chip->flags |= TPM_CHIP_FLAG_ALWAYS_POWERED;
> +

This last line here blows up the build, there is no chip variable
defined in this function :(

So I have to drop both of these patches, from both 4.4.y and 4.9.y
queues right now.  Can you fix this up and resend them?

thanks,

greg k-h

Re: [PATCH v2] clk: x86: Add ST oscout platform clock

[Stephen Boyd]

Quoting Akshu Agrawal (2018-05-03 01:30:26)
> diff --git a/drivers/clk/x86/Makefile b/drivers/clk/x86/Makefile
> index 1367afb..2aee002 100644
> --- a/drivers/clk/x86/Makefile
> +++ b/drivers/clk/x86/Makefile
> @@ -1,3 +1,4 @@
>  clk-x86-lpss-objs  := clk-lpt.o
>  obj-$(CONFIG_X86_INTEL_LPSS)   += clk-x86-lpss.o
>  obj-$(CONFIG_PMC_ATOM) += clk-pmc-atom.o
> +obj-$(CONFIG_X86_AMD_PLATFORM_DEVICE)  += clk-st.o

Ok. Can you sort this by kconfig? Or by file name?

> diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c
> new file mode 100644
> index 000..d8c283c
> --- /dev/null
> +++ b/drivers/clk/x86/clk-st.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * clock framework for AMD Stoney based clocks
> + *
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.

One point of SPDX is to avoid this boiler plate multi-line license
comments. Can you remove this and just leave the AMD copyright part?

> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Clock Driving Strength 2 register */
> +#define CLKDRVSTR2 0x28
> +/* Clock Control 1 register */
> +#define MISCCLKCNTL1   0x40
> +/* Auxiliary clock1 enable bit */
> +#define OSCCLKENB  2
> +/* 25Mhz auxiliary output clock freq bit */
> +#define OSCOUT1CLK25MHZ16
> +
> +#define ST_CLK_48M 0
> +#define ST_CLK_25M 1
> +#define ST_CLK_MUX 2
> +#define ST_CLK_GATE3
> +#define ST_MAX_CLKS4
> +
> +static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" };
> +
> +static int st_clk_probe(struct platform_device *pdev)
> +{
> +   struct st_clk_data *st_data;
> +   struct clk_hw **hws;
> +
> +   st_data = dev_get_platdata(>dev);
> +   if (!st_data || !st_data->base)
> +   return -EINVAL;
> +
> +   hws = kzalloc(sizeof(*hws) * ST_MAX_CLKS, GFP_KERNEL);

Fix the kbuild robot errors please.

> +
> +   hws[ST_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz", NULL, 
> 0,
> +4800);
> +   hws[ST_CLK_25M] = clk_hw_register_fixed_rate(NULL, "clk25MHz", NULL, 
> 0,
> +2500);

I'm not sure why we even keep these pointers around though. The driver
doesn't expose them as clks that clk_get() can find so they could just
be local variables and no heap allocation is needed.

> +
> +   hws[ST_CLK_MUX] = clk_hw_register_mux(NULL, "oscout1_mux",
> +   clk_oscout1_parents, ARRAY_SIZE(clk_oscout1_parents),
> +   0, st_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3, 0, NULL);
> +
> +   clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_25M]->clk);
> +
> +   hws[ST_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1", 
> "oscout1_mux",
> +   0, st_data->base + MISCCLKCNTL1, OSCCLKENB,
> +   CLK_GATE_SET_TO_DISABLE, NULL);
> +
> +   clk_hw_register_clkdev(hws[ST_CLK_GATE], "oscout1", NULL);

Could use devm_*() here in case you want to drop this stuff on driver
removal?

> +
> +   return 0;
> +}
> +
> +static struct platform_driver st_clk_driver = {
> +   .driver = {
> +   .name = "clk-st",
> +   },
> +   .probe = st_clk_probe,

suppress attributes here to prevent unbinding from sysfs?

> +};
> +builtin_platform_driver(st_clk_driver);

Re: [PATCH v2] clk: x86: Add ST oscout platform clock

[Stephen Boyd]

Quoting Akshu Agrawal (2018-05-03 01:30:26)
> diff --git a/drivers/clk/x86/Makefile b/drivers/clk/x86/Makefile
> index 1367afb..2aee002 100644
> --- a/drivers/clk/x86/Makefile
> +++ b/drivers/clk/x86/Makefile
> @@ -1,3 +1,4 @@
>  clk-x86-lpss-objs  := clk-lpt.o
>  obj-$(CONFIG_X86_INTEL_LPSS)   += clk-x86-lpss.o
>  obj-$(CONFIG_PMC_ATOM) += clk-pmc-atom.o
> +obj-$(CONFIG_X86_AMD_PLATFORM_DEVICE)  += clk-st.o

Ok. Can you sort this by kconfig? Or by file name?

> diff --git a/drivers/clk/x86/clk-st.c b/drivers/clk/x86/clk-st.c
> new file mode 100644
> index 000..d8c283c
> --- /dev/null
> +++ b/drivers/clk/x86/clk-st.c
> @@ -0,0 +1,86 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * clock framework for AMD Stoney based clocks
> + *
> + * Copyright 2018 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.

One point of SPDX is to avoid this boiler plate multi-line license
comments. Can you remove this and just leave the AMD copyright part?

> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* Clock Driving Strength 2 register */
> +#define CLKDRVSTR2 0x28
> +/* Clock Control 1 register */
> +#define MISCCLKCNTL1   0x40
> +/* Auxiliary clock1 enable bit */
> +#define OSCCLKENB  2
> +/* 25Mhz auxiliary output clock freq bit */
> +#define OSCOUT1CLK25MHZ16
> +
> +#define ST_CLK_48M 0
> +#define ST_CLK_25M 1
> +#define ST_CLK_MUX 2
> +#define ST_CLK_GATE3
> +#define ST_MAX_CLKS4
> +
> +static const char * const clk_oscout1_parents[] = { "clk48MHz", "clk25MHz" };
> +
> +static int st_clk_probe(struct platform_device *pdev)
> +{
> +   struct st_clk_data *st_data;
> +   struct clk_hw **hws;
> +
> +   st_data = dev_get_platdata(>dev);
> +   if (!st_data || !st_data->base)
> +   return -EINVAL;
> +
> +   hws = kzalloc(sizeof(*hws) * ST_MAX_CLKS, GFP_KERNEL);

Fix the kbuild robot errors please.

> +
> +   hws[ST_CLK_48M] = clk_hw_register_fixed_rate(NULL, "clk48MHz", NULL, 
> 0,
> +4800);
> +   hws[ST_CLK_25M] = clk_hw_register_fixed_rate(NULL, "clk25MHz", NULL, 
> 0,
> +2500);

I'm not sure why we even keep these pointers around though. The driver
doesn't expose them as clks that clk_get() can find so they could just
be local variables and no heap allocation is needed.

> +
> +   hws[ST_CLK_MUX] = clk_hw_register_mux(NULL, "oscout1_mux",
> +   clk_oscout1_parents, ARRAY_SIZE(clk_oscout1_parents),
> +   0, st_data->base + CLKDRVSTR2, OSCOUT1CLK25MHZ, 3, 0, NULL);
> +
> +   clk_set_parent(hws[ST_CLK_MUX]->clk, hws[ST_CLK_25M]->clk);
> +
> +   hws[ST_CLK_GATE] = clk_hw_register_gate(NULL, "oscout1", 
> "oscout1_mux",
> +   0, st_data->base + MISCCLKCNTL1, OSCCLKENB,
> +   CLK_GATE_SET_TO_DISABLE, NULL);
> +
> +   clk_hw_register_clkdev(hws[ST_CLK_GATE], "oscout1", NULL);

Could use devm_*() here in case you want to drop this stuff on driver
removal?

> +
> +   return 0;
> +}
> +
> +static struct platform_driver st_clk_driver = {
> +   .driver = {
> +   .name = "clk-st",
> +   },
> +   .probe = st_clk_probe,

suppress attributes here to prevent unbinding from sysfs?

> +};
> +builtin_platform_driver(st_clk_driver);

[RESEND PATCH 5/5] usb: mtu3: make USB_MTU3_DUAL_ROLE depend on EXTCON but not USB_MTU3

[Chunfeng Yun]

In fact the driver depends on EXTCON only when it's configed as
USB_MTU3_DUAL_ROLE, so make USB_MTU3_DUAL_ROLE depend on EXTCON but
not USB_MTU3.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
index 25cd619..8daf277 100644
--- a/drivers/usb/mtu3/Kconfig
+++ b/drivers/usb/mtu3/Kconfig
@@ -2,7 +2,7 @@
 
 config USB_MTU3
tristate "MediaTek USB3 Dual Role controller"
-   depends on EXTCON && (USB || USB_GADGET) && HAS_DMA
+   depends on (USB || USB_GADGET) && HAS_DMA
depends on ARCH_MEDIATEK || COMPILE_TEST
select USB_XHCI_MTK if USB_SUPPORT && USB_XHCI_HCD
help
@@ -40,6 +40,7 @@ config USB_MTU3_GADGET
 config USB_MTU3_DUAL_ROLE
bool "Dual Role mode"
depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || 
USB_GADGET=USB_MTU3))
+   depends on (EXTCON=y || EXTCON=USB_MTU3)
help
  This is the default mode of working of MTU3 controller where
  both host and gadget features are enabled.
-- 
1.9.1

[RESEND PATCH 1/5] usb: mtu3: avoid TX data length truncated in SS/SSP mode

[Chunfeng Yun]

The variable of 'count' is declared as u8, this will cause an issue
due to value truncated when works in SS or SSP mode and data length
is greater than 255, so change it as u32.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3_gadget_ep0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c 
b/drivers/usb/mtu3/mtu3_gadget_ep0.c
index ebdcf7a..d67b540 100644
--- a/drivers/usb/mtu3/mtu3_gadget_ep0.c
+++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c
@@ -546,7 +546,7 @@ static void ep0_tx_state(struct mtu3 *mtu)
struct usb_request *req;
u32 csr;
u8 *src;
-   u8 count;
+   u32 count;
u32 maxp;
 
dev_dbg(mtu->dev, "%s\n", __func__);
-- 
1.9.1

[RESEND PATCH 4/5] usb: mtu3: fix operation failure when test TEST_J/K

[Chunfeng Yun]

There is an error dialog popped up in PC when test TEST_J/K
by EHSETT tool, due to not waiting for the completion of
control transfer. Here fix it by entering test mode after
Status Stage finish.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3_gadget_ep0.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c 
b/drivers/usb/mtu3/mtu3_gadget_ep0.c
index d67b540..0d2b1cf 100644
--- a/drivers/usb/mtu3/mtu3_gadget_ep0.c
+++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c
@@ -7,6 +7,7 @@
  * Author:  Chunfeng.Yun 
  */
 
+#include 
 #include 
 
 #include "mtu3.h"
@@ -263,6 +264,7 @@ static int handle_test_mode(struct mtu3 *mtu, struct 
usb_ctrlrequest *setup)
 {
void __iomem *mbase = mtu->mac_base;
int handled = 1;
+   u32 value;
 
switch (le16_to_cpu(setup->wIndex) >> 8) {
case TEST_J:
@@ -292,6 +294,14 @@ static int handle_test_mode(struct mtu3 *mtu, struct 
usb_ctrlrequest *setup)
if (mtu->test_mode_nr == TEST_PACKET_MODE)
ep0_load_test_packet(mtu);
 
+   /* send status before entering test mode. */
+   value = mtu3_readl(mbase, U3D_EP0CSR) & EP0_W1C_BITS;
+   mtu3_writel(mbase, U3D_EP0CSR, value | EP0_SETUPPKTRDY | EP0_DATAEND);
+
+   /* wait for ACK status sent by host */
+   readl_poll_timeout(mbase + U3D_EP0CSR, value,
+   !(value & EP0_DATAEND), 100, 5000);
+
mtu3_writel(mbase, U3D_USB2_TEST_MODE, mtu->test_mode_nr);
 
mtu->ep0_state = MU3D_EP0_STATE_SETUP;
-- 
1.9.1

[RESEND PATCH 5/5] usb: mtu3: make USB_MTU3_DUAL_ROLE depend on EXTCON but not USB_MTU3

[Chunfeng Yun]

In fact the driver depends on EXTCON only when it's configed as
USB_MTU3_DUAL_ROLE, so make USB_MTU3_DUAL_ROLE depend on EXTCON but
not USB_MTU3.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/Kconfig b/drivers/usb/mtu3/Kconfig
index 25cd619..8daf277 100644
--- a/drivers/usb/mtu3/Kconfig
+++ b/drivers/usb/mtu3/Kconfig
@@ -2,7 +2,7 @@
 
 config USB_MTU3
tristate "MediaTek USB3 Dual Role controller"
-   depends on EXTCON && (USB || USB_GADGET) && HAS_DMA
+   depends on (USB || USB_GADGET) && HAS_DMA
depends on ARCH_MEDIATEK || COMPILE_TEST
select USB_XHCI_MTK if USB_SUPPORT && USB_XHCI_HCD
help
@@ -40,6 +40,7 @@ config USB_MTU3_GADGET
 config USB_MTU3_DUAL_ROLE
bool "Dual Role mode"
depends on ((USB=y || USB=USB_MTU3) && (USB_GADGET=y || 
USB_GADGET=USB_MTU3))
+   depends on (EXTCON=y || EXTCON=USB_MTU3)
help
  This is the default mode of working of MTU3 controller where
  both host and gadget features are enabled.
-- 
1.9.1

[RESEND PATCH 1/5] usb: mtu3: avoid TX data length truncated in SS/SSP mode

[Chunfeng Yun]

The variable of 'count' is declared as u8, this will cause an issue
due to value truncated when works in SS or SSP mode and data length
is greater than 255, so change it as u32.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3_gadget_ep0.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c 
b/drivers/usb/mtu3/mtu3_gadget_ep0.c
index ebdcf7a..d67b540 100644
--- a/drivers/usb/mtu3/mtu3_gadget_ep0.c
+++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c
@@ -546,7 +546,7 @@ static void ep0_tx_state(struct mtu3 *mtu)
struct usb_request *req;
u32 csr;
u8 *src;
-   u8 count;
+   u32 count;
u32 maxp;
 
dev_dbg(mtu->dev, "%s\n", __func__);
-- 
1.9.1

[RESEND PATCH 4/5] usb: mtu3: fix operation failure when test TEST_J/K

[Chunfeng Yun]

There is an error dialog popped up in PC when test TEST_J/K
by EHSETT tool, due to not waiting for the completion of
control transfer. Here fix it by entering test mode after
Status Stage finish.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3_gadget_ep0.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c 
b/drivers/usb/mtu3/mtu3_gadget_ep0.c
index d67b540..0d2b1cf 100644
--- a/drivers/usb/mtu3/mtu3_gadget_ep0.c
+++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c
@@ -7,6 +7,7 @@
  * Author:  Chunfeng.Yun 
  */
 
+#include 
 #include 
 
 #include "mtu3.h"
@@ -263,6 +264,7 @@ static int handle_test_mode(struct mtu3 *mtu, struct 
usb_ctrlrequest *setup)
 {
void __iomem *mbase = mtu->mac_base;
int handled = 1;
+   u32 value;
 
switch (le16_to_cpu(setup->wIndex) >> 8) {
case TEST_J:
@@ -292,6 +294,14 @@ static int handle_test_mode(struct mtu3 *mtu, struct 
usb_ctrlrequest *setup)
if (mtu->test_mode_nr == TEST_PACKET_MODE)
ep0_load_test_packet(mtu);
 
+   /* send status before entering test mode. */
+   value = mtu3_readl(mbase, U3D_EP0CSR) & EP0_W1C_BITS;
+   mtu3_writel(mbase, U3D_EP0CSR, value | EP0_SETUPPKTRDY | EP0_DATAEND);
+
+   /* wait for ACK status sent by host */
+   readl_poll_timeout(mbase + U3D_EP0CSR, value,
+   !(value & EP0_DATAEND), 100, 5000);
+
mtu3_writel(mbase, U3D_USB2_TEST_MODE, mtu->test_mode_nr);
 
mtu->ep0_state = MU3D_EP0_STATE_SETUP;
-- 
1.9.1

[RESEND PATCH 3/5] usb: mtu3: fix an unrecognized issue when connected with PC

[Chunfeng Yun]

When boot on the platform with the USB cable connected to Win7,
the Win7 will pop up an error dialog: "USB Device not recognized",
but finally the Win7 can enumerate it successfully.
The root cause is as the following:
When the xHCI driver set PORT_POWER of the OTG port, and if both
IDPIN and VBUS_VALID are high at the same time, the MTU3 controller
will set SESSION and pull up DP, so the Win7 can detect existence
of USB device, but if the mtu3 driver can't switch to device mode
during the debounce time, the Win7 can not enumerate it.
Here to fix it by removing the 1s delayed EXTCON register to speed up
mode switch.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3.h|  4 
 drivers/usb/mtu3/mtu3_dr.c | 25 +++--
 2 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index 2cd00a2..a56fee0 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -197,9 +197,6 @@ struct mtu3_gpd_ring {
 * @edev: external connector used to detect vbus and iddig changes
 * @vbus_nb: notifier for vbus detection
 * @vbus_nb: notifier for iddig(idpin) detection
-* @extcon_reg_dwork: delay work for extcon notifier register, waiting for
-*  xHCI driver initialization, it's necessary for system bootup
-*  as device.
 * @is_u3_drd: whether port0 supports usb3.0 dual-role device or not
 * @manual_drd_enabled: it's true when supports dual-role device by debugfs
 *  to switch host/device modes depending on user input.
@@ -209,7 +206,6 @@ struct otg_switch_mtk {
struct extcon_dev *edev;
struct notifier_block vbus_nb;
struct notifier_block id_nb;
-   struct delayed_work extcon_reg_dwork;
bool is_u3_drd;
bool manual_drd_enabled;
 };
diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
index db7562d..80083e0 100644
--- a/drivers/usb/mtu3/mtu3_dr.c
+++ b/drivers/usb/mtu3/mtu3_dr.c
@@ -238,15 +238,6 @@ static int ssusb_extcon_register(struct otg_switch_mtk 
*otg_sx)
return 0;
 }
 
-static void extcon_register_dwork(struct work_struct *work)
-{
-   struct delayed_work *dwork = to_delayed_work(work);
-   struct otg_switch_mtk *otg_sx =
-   container_of(dwork, struct otg_switch_mtk, extcon_reg_dwork);
-
-   ssusb_extcon_register(otg_sx);
-}
-
 /*
  * We provide an interface via debugfs to switch between host and device modes
  * depending on user input.
@@ -407,18 +398,10 @@ int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)
 {
struct otg_switch_mtk *otg_sx = >otg_switch;
 
-   if (otg_sx->manual_drd_enabled) {
+   if (otg_sx->manual_drd_enabled)
ssusb_debugfs_init(ssusb);
-   } else {
-   INIT_DELAYED_WORK(_sx->extcon_reg_dwork,
- extcon_register_dwork);
-
-   /*
-* It is enough to delay 1s for waiting for
-* host initialization
-*/
-   schedule_delayed_work(_sx->extcon_reg_dwork, HZ);
-   }
+   else
+   ssusb_extcon_register(otg_sx);
 
return 0;
 }
@@ -429,6 +412,4 @@ void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb)
 
if (otg_sx->manual_drd_enabled)
ssusb_debugfs_exit(ssusb);
-   else
-   cancel_delayed_work(_sx->extcon_reg_dwork);
 }
-- 
1.9.1

[RESEND PATCH 3/5] usb: mtu3: fix an unrecognized issue when connected with PC

[Chunfeng Yun]

When boot on the platform with the USB cable connected to Win7,
the Win7 will pop up an error dialog: "USB Device not recognized",
but finally the Win7 can enumerate it successfully.
The root cause is as the following:
When the xHCI driver set PORT_POWER of the OTG port, and if both
IDPIN and VBUS_VALID are high at the same time, the MTU3 controller
will set SESSION and pull up DP, so the Win7 can detect existence
of USB device, but if the mtu3 driver can't switch to device mode
during the debounce time, the Win7 can not enumerate it.
Here to fix it by removing the 1s delayed EXTCON register to speed up
mode switch.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3.h|  4 
 drivers/usb/mtu3/mtu3_dr.c | 25 +++--
 2 files changed, 3 insertions(+), 26 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3.h b/drivers/usb/mtu3/mtu3.h
index 2cd00a2..a56fee0 100644
--- a/drivers/usb/mtu3/mtu3.h
+++ b/drivers/usb/mtu3/mtu3.h
@@ -197,9 +197,6 @@ struct mtu3_gpd_ring {
 * @edev: external connector used to detect vbus and iddig changes
 * @vbus_nb: notifier for vbus detection
 * @vbus_nb: notifier for iddig(idpin) detection
-* @extcon_reg_dwork: delay work for extcon notifier register, waiting for
-*  xHCI driver initialization, it's necessary for system bootup
-*  as device.
 * @is_u3_drd: whether port0 supports usb3.0 dual-role device or not
 * @manual_drd_enabled: it's true when supports dual-role device by debugfs
 *  to switch host/device modes depending on user input.
@@ -209,7 +206,6 @@ struct otg_switch_mtk {
struct extcon_dev *edev;
struct notifier_block vbus_nb;
struct notifier_block id_nb;
-   struct delayed_work extcon_reg_dwork;
bool is_u3_drd;
bool manual_drd_enabled;
 };
diff --git a/drivers/usb/mtu3/mtu3_dr.c b/drivers/usb/mtu3/mtu3_dr.c
index db7562d..80083e0 100644
--- a/drivers/usb/mtu3/mtu3_dr.c
+++ b/drivers/usb/mtu3/mtu3_dr.c
@@ -238,15 +238,6 @@ static int ssusb_extcon_register(struct otg_switch_mtk 
*otg_sx)
return 0;
 }
 
-static void extcon_register_dwork(struct work_struct *work)
-{
-   struct delayed_work *dwork = to_delayed_work(work);
-   struct otg_switch_mtk *otg_sx =
-   container_of(dwork, struct otg_switch_mtk, extcon_reg_dwork);
-
-   ssusb_extcon_register(otg_sx);
-}
-
 /*
  * We provide an interface via debugfs to switch between host and device modes
  * depending on user input.
@@ -407,18 +398,10 @@ int ssusb_otg_switch_init(struct ssusb_mtk *ssusb)
 {
struct otg_switch_mtk *otg_sx = >otg_switch;
 
-   if (otg_sx->manual_drd_enabled) {
+   if (otg_sx->manual_drd_enabled)
ssusb_debugfs_init(ssusb);
-   } else {
-   INIT_DELAYED_WORK(_sx->extcon_reg_dwork,
- extcon_register_dwork);
-
-   /*
-* It is enough to delay 1s for waiting for
-* host initialization
-*/
-   schedule_delayed_work(_sx->extcon_reg_dwork, HZ);
-   }
+   else
+   ssusb_extcon_register(otg_sx);
 
return 0;
 }
@@ -429,6 +412,4 @@ void ssusb_otg_switch_exit(struct ssusb_mtk *ssusb)
 
if (otg_sx->manual_drd_enabled)
ssusb_debugfs_exit(ssusb);
-   else
-   cancel_delayed_work(_sx->extcon_reg_dwork);
 }
-- 
1.9.1

Re: [PATCH v2] f2fs: fix to avoid race during access gc_thread pointer

[Chao Yu]

On 2018/5/5 2:59, Jaegeuk Kim wrote:
> On 05/02, Chao Yu wrote:
>> On 2018/4/28 10:36, Jaegeuk Kim wrote:
>>> On 04/27, Chao Yu wrote:
 On 2018/4/27 0:03, Jaegeuk Kim wrote:
> On 04/24, Chao Yu wrote:
>> Thread A Thread BThread C
>> - f2fs_remount
>>  - stop_gc_thread
>>  - f2fs_sbi_store
>>  - issue_discard_thread
>>sbi->gc_thread = NULL;
>>sbi->gc_thread->gc_wake = 1
>>access 
>> sbi->gc_thread->gc_urgent
>>
>> Previously, we allocate memory for sbi->gc_thread based on background
>> gc thread mount option, the memory can be released if we turn off
>> that mount option, but still there are several places access gc_thread
>> pointer without considering race condition, result in NULL point
>> dereference.
>
> Do we just need to check >s_umount in f2fs_sbi_store() and

 Better,

> issue_discard_thread?

 There is more cases can be called from different scenarios:
 - select_policy()
 - need_SSR()
>>>
>>> No? They should be blocked by remount(2).
>>
>> How about below cases?
>>
>> - do_remount
> 
> Was there no guard to block any filesystem operations?

The only block point is f2fs_readonly in f2fs_sync_file, without holding
s_umount during ->fsync, so IMO, we need to cover need_SSR & select_gc_type, let
me update the patch later.

Thanks,

> If it's true, yeah, we need to cover them.
> 
>>  - do_remount_sb
>>   - remount_fs
>>- f2fs_remount
>> - stop_gc_thread
>>  - fsync
>>   - f2fs_sync_file
>>- file_write_and_wait_range
>> - f2fs_write_data_pages
>>  - __write_data_page
>>   - should_update_inplace
>>- check_inplace_update_policy
>> - need_SSR
>>  : sbi->gc_thread = NULL;
>>
>> or
>>
>>   - write_data_page
>>- allocate_data_block
>> - allocate_segment
>>  - get_ssr_segment
>>   - select_gc_type
>>  : sbi->gc_thread = NULL;
>>
>> Thanks,
>>
>>>

 Thanks,

>
>>
>> In order to fix this issue, keep gc_thread structure valid in sbi all
>> the time instead of alloc/free it dynamically.
>>
>> In addition, add a rw semaphore to exclude rw operation in fields of
>> gc_thread.
>>
>> Signed-off-by: Chao Yu 
>> ---
>> v2:
>> - add a rw semaphore to exclude rw operation suggested by Jaegeuk.
>>  fs/f2fs/debug.c   |  3 +--
>>  fs/f2fs/f2fs.h|  9 ++-
>>  fs/f2fs/gc.c  | 78 
>> ---
>>  fs/f2fs/gc.h  |  1 +
>>  fs/f2fs/segment.c | 10 +--
>>  fs/f2fs/super.c   | 26 +--
>>  fs/f2fs/sysfs.c   | 26 ++-
>>  7 files changed, 107 insertions(+), 46 deletions(-)
>>
>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>> index a66107b5cfff..0fbd674c66fb 100644
>> --- a/fs/f2fs/debug.c
>> +++ b/fs/f2fs/debug.c
>> @@ -221,8 +221,7 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
>>  si->cache_mem = 0;
>>  
>>  /* build gc */
>> -if (sbi->gc_thread)
>> -si->cache_mem += sizeof(struct f2fs_gc_kthread);
>> +si->cache_mem += sizeof(struct f2fs_gc_kthread);
>>  
>>  /* build merge flush thread */
>>  if (SM_I(sbi)->fcc_info)
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 8f3ad9662d13..75d3b4875429 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1412,6 +1412,11 @@ static inline struct sit_info *SIT_I(struct 
>> f2fs_sb_info *sbi)
>>  return (struct sit_info *)(SM_I(sbi)->sit_info);
>>  }
>>  
>> +static inline struct f2fs_gc_kthread *GC_I(struct f2fs_sb_info *sbi)
>> +{
>> +return (struct f2fs_gc_kthread *)(sbi->gc_thread);
>> +}
>> +
>>  static inline struct free_segmap_info *FREE_I(struct f2fs_sb_info *sbi)
>>  {
>>  return (struct free_segmap_info *)(SM_I(sbi)->free_info);
>> @@ -2954,8 +2959,10 @@ bool f2fs_overwrite_io(struct inode *inode, 
>> loff_t pos, size_t len);
>>  /*
>>   * gc.c
>>   */
>> +int init_gc_context(struct f2fs_sb_info *sbi);
>> +void destroy_gc_context(struct f2fs_sb_info * sbi);
>>  int 

[RESEND PATCH 2/5] usb: mtu3: remove repeated setting of gadget state

[Chunfeng Yun]

The usb_add_gadget_udc() will set the gadget state as
USB_STATE_NOTATTACHED, so we needn't set it again.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3_gadget.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
index f05f10f..de0de01 100644
--- a/drivers/usb/mtu3/mtu3_gadget.c
+++ b/drivers/usb/mtu3/mtu3_gadget.c
@@ -660,14 +660,10 @@ int mtu3_gadget_setup(struct mtu3 *mtu)
mtu3_gadget_init_eps(mtu);
 
ret = usb_add_gadget_udc(mtu->dev, >g);
-   if (ret) {
+   if (ret)
dev_err(mtu->dev, "failed to register udc\n");
-   return ret;
-   }
 
-   usb_gadget_set_state(>g, USB_STATE_NOTATTACHED);
-
-   return 0;
+   return ret;
 }
 
 void mtu3_gadget_cleanup(struct mtu3 *mtu)
-- 
1.9.1

Re: [PATCH v2] f2fs: fix to avoid race during access gc_thread pointer

[Chao Yu]

On 2018/5/5 2:59, Jaegeuk Kim wrote:
> On 05/02, Chao Yu wrote:
>> On 2018/4/28 10:36, Jaegeuk Kim wrote:
>>> On 04/27, Chao Yu wrote:
 On 2018/4/27 0:03, Jaegeuk Kim wrote:
> On 04/24, Chao Yu wrote:
>> Thread A Thread BThread C
>> - f2fs_remount
>>  - stop_gc_thread
>>  - f2fs_sbi_store
>>  - issue_discard_thread
>>sbi->gc_thread = NULL;
>>sbi->gc_thread->gc_wake = 1
>>access 
>> sbi->gc_thread->gc_urgent
>>
>> Previously, we allocate memory for sbi->gc_thread based on background
>> gc thread mount option, the memory can be released if we turn off
>> that mount option, but still there are several places access gc_thread
>> pointer without considering race condition, result in NULL point
>> dereference.
>
> Do we just need to check >s_umount in f2fs_sbi_store() and

 Better,

> issue_discard_thread?

 There is more cases can be called from different scenarios:
 - select_policy()
 - need_SSR()
>>>
>>> No? They should be blocked by remount(2).
>>
>> How about below cases?
>>
>> - do_remount
> 
> Was there no guard to block any filesystem operations?

The only block point is f2fs_readonly in f2fs_sync_file, without holding
s_umount during ->fsync, so IMO, we need to cover need_SSR & select_gc_type, let
me update the patch later.

Thanks,

> If it's true, yeah, we need to cover them.
> 
>>  - do_remount_sb
>>   - remount_fs
>>- f2fs_remount
>> - stop_gc_thread
>>  - fsync
>>   - f2fs_sync_file
>>- file_write_and_wait_range
>> - f2fs_write_data_pages
>>  - __write_data_page
>>   - should_update_inplace
>>- check_inplace_update_policy
>> - need_SSR
>>  : sbi->gc_thread = NULL;
>>
>> or
>>
>>   - write_data_page
>>- allocate_data_block
>> - allocate_segment
>>  - get_ssr_segment
>>   - select_gc_type
>>  : sbi->gc_thread = NULL;
>>
>> Thanks,
>>
>>>

 Thanks,

>
>>
>> In order to fix this issue, keep gc_thread structure valid in sbi all
>> the time instead of alloc/free it dynamically.
>>
>> In addition, add a rw semaphore to exclude rw operation in fields of
>> gc_thread.
>>
>> Signed-off-by: Chao Yu 
>> ---
>> v2:
>> - add a rw semaphore to exclude rw operation suggested by Jaegeuk.
>>  fs/f2fs/debug.c   |  3 +--
>>  fs/f2fs/f2fs.h|  9 ++-
>>  fs/f2fs/gc.c  | 78 
>> ---
>>  fs/f2fs/gc.h  |  1 +
>>  fs/f2fs/segment.c | 10 +--
>>  fs/f2fs/super.c   | 26 +--
>>  fs/f2fs/sysfs.c   | 26 ++-
>>  7 files changed, 107 insertions(+), 46 deletions(-)
>>
>> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
>> index a66107b5cfff..0fbd674c66fb 100644
>> --- a/fs/f2fs/debug.c
>> +++ b/fs/f2fs/debug.c
>> @@ -221,8 +221,7 @@ static void update_mem_info(struct f2fs_sb_info *sbi)
>>  si->cache_mem = 0;
>>  
>>  /* build gc */
>> -if (sbi->gc_thread)
>> -si->cache_mem += sizeof(struct f2fs_gc_kthread);
>> +si->cache_mem += sizeof(struct f2fs_gc_kthread);
>>  
>>  /* build merge flush thread */
>>  if (SM_I(sbi)->fcc_info)
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index 8f3ad9662d13..75d3b4875429 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -1412,6 +1412,11 @@ static inline struct sit_info *SIT_I(struct 
>> f2fs_sb_info *sbi)
>>  return (struct sit_info *)(SM_I(sbi)->sit_info);
>>  }
>>  
>> +static inline struct f2fs_gc_kthread *GC_I(struct f2fs_sb_info *sbi)
>> +{
>> +return (struct f2fs_gc_kthread *)(sbi->gc_thread);
>> +}
>> +
>>  static inline struct free_segmap_info *FREE_I(struct f2fs_sb_info *sbi)
>>  {
>>  return (struct free_segmap_info *)(SM_I(sbi)->free_info);
>> @@ -2954,8 +2959,10 @@ bool f2fs_overwrite_io(struct inode *inode, 
>> loff_t pos, size_t len);
>>  /*
>>   * gc.c
>>   */
>> +int init_gc_context(struct f2fs_sb_info *sbi);
>> +void destroy_gc_context(struct f2fs_sb_info * sbi);
>>  int start_gc_thread(struct 

[RESEND PATCH 2/5] usb: mtu3: remove repeated setting of gadget state

[Chunfeng Yun]

The usb_add_gadget_udc() will set the gadget state as
USB_STATE_NOTATTACHED, so we needn't set it again.

Signed-off-by: Chunfeng Yun 
---
 drivers/usb/mtu3/mtu3_gadget.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget.c b/drivers/usb/mtu3/mtu3_gadget.c
index f05f10f..de0de01 100644
--- a/drivers/usb/mtu3/mtu3_gadget.c
+++ b/drivers/usb/mtu3/mtu3_gadget.c
@@ -660,14 +660,10 @@ int mtu3_gadget_setup(struct mtu3 *mtu)
mtu3_gadget_init_eps(mtu);
 
ret = usb_add_gadget_udc(mtu->dev, >g);
-   if (ret) {
+   if (ret)
dev_err(mtu->dev, "failed to register udc\n");
-   return ret;
-   }
 
-   usb_gadget_set_state(>g, USB_STATE_NOTATTACHED);
-
-   return 0;
+   return ret;
 }
 
 void mtu3_gadget_cleanup(struct mtu3 *mtu)
-- 
1.9.1

[PATCH v2 2/2] phy: mediatek: add XS-PHY driver

[Chunfeng Yun]

Support XS-PHY for MediaTek SoCs with USB3.1 GEN2 controller

Signed-off-by: Chunfeng Yun 
---
 drivers/phy/mediatek/Kconfig |9 +
 drivers/phy/mediatek/Makefile|1 +
 drivers/phy/mediatek/phy-mtk-xsphy.c |  600 ++
 3 files changed, 610 insertions(+)
 create mode 100644 drivers/phy/mediatek/phy-mtk-xsphy.c

diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 88ab4e2..8857d00 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -12,3 +12,12 @@ config PHY_MTK_TPHY
  different banks layout, the T-PHY with shared banks between
  multi-ports is first version, otherwise is second veriosn,
  so you can easily distinguish them by banks layout.
+
+config PHY_MTK_XSPHY
+tristate "MediaTek XS-PHY Driver"
+depends on ARCH_MEDIATEK && OF
+select GENERIC_PHY
+help
+ Enable this to support the SuperSpeedPlus XS-PHY transceiver for
+ USB3.1 GEN2 controllers on MediaTek chips. The driver supports
+ multiple USB2.0, USB3.1 GEN2 ports.
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index 1bdab14..ee49edc 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -4,3 +4,4 @@
 #
 
 obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
+obj-$(CONFIG_PHY_MTK_XSPHY)+= phy-mtk-xsphy.o
diff --git a/drivers/phy/mediatek/phy-mtk-xsphy.c 
b/drivers/phy/mediatek/phy-mtk-xsphy.c
new file mode 100644
index 000..020cd02
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-xsphy.c
@@ -0,0 +1,600 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek USB3.1 gen2 xsphy Driver
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Chunfeng Yun 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* u2 phy banks */
+#define SSUSB_SIFSLV_MISC  0x000
+#define SSUSB_SIFSLV_U2FREQ0x100
+#define SSUSB_SIFSLV_U2PHY_COM 0x300
+
+/* u3 phy shared banks */
+#define SSPXTP_SIFSLV_DIG_GLB  0x000
+#define SSPXTP_SIFSLV_PHYA_GLB 0x100
+
+/* u3 phy banks */
+#define SSPXTP_SIFSLV_DIG_LN_TOP   0x000
+#define SSPXTP_SIFSLV_DIG_LN_TX0   0x100
+#define SSPXTP_SIFSLV_DIG_LN_RX0   0x200
+#define SSPXTP_SIFSLV_DIG_LN_DAIF  0x300
+#define SSPXTP_SIFSLV_PHYA_LN  0x400
+
+#define XSP_U2FREQ_FMCR0   ((SSUSB_SIFSLV_U2FREQ) + 0x00)
+#define P2F_RG_FREQDET_EN  BIT(24)
+#define P2F_RG_CYCLECNTGENMASK(23, 0)
+#define P2F_RG_CYCLECNT_VAL(x) ((P2F_RG_CYCLECNT) & (x))
+
+#define XSP_U2FREQ_MMONR0  ((SSUSB_SIFSLV_U2FREQ) + 0x0c)
+
+#define XSP_U2FREQ_FMMONR1 ((SSUSB_SIFSLV_U2FREQ) + 0x10)
+#define P2F_RG_FRCK_EN BIT(8)
+#define P2F_USB_FM_VALID   BIT(0)
+
+#define XSP_USBPHYACR0 ((SSUSB_SIFSLV_U2PHY_COM) + 0x00)
+#define P2A0_RG_INTR_ENBIT(5)
+
+#define XSP_USBPHYACR1 ((SSUSB_SIFSLV_U2PHY_COM) + 0x04)
+#define P2A1_RG_INTR_CAL   GENMASK(23, 19)
+#define P2A1_RG_INTR_CAL_VAL(x)((0x1f & (x)) << 19)
+#define P2A1_RG_VRT_SELGENMASK(14, 12)
+#define P2A1_RG_VRT_SEL_VAL(x) ((0x7 & (x)) << 12)
+#define P2A1_RG_TERM_SEL   GENMASK(10, 8)
+#define P2A1_RG_TERM_SEL_VAL(x)((0x7 & (x)) << 8)
+
+#define XSP_USBPHYACR5 ((SSUSB_SIFSLV_U2PHY_COM) + 0x014)
+#define P2A5_RG_HSTX_SRCAL_EN  BIT(15)
+#define P2A5_RG_HSTX_SRCTRLGENMASK(14, 12)
+#define P2A5_RG_HSTX_SRCTRL_VAL(x) ((0x7 & (x)) << 12)
+
+#define XSP_USBPHYACR6 ((SSUSB_SIFSLV_U2PHY_COM) + 0x018)
+#define P2A6_RG_BC11_SW_EN BIT(23)
+#define P2A6_RG_OTG_VBUSCMP_EN BIT(20)
+
+#define XSP_U2PHYDTM1  ((SSUSB_SIFSLV_U2PHY_COM) + 0x06C)
+#define P2D_FORCE_IDDIGBIT(9)
+#define P2D_RG_VBUSVALID   BIT(5)
+#define P2D_RG_SESSEND BIT(4)
+#define P2D_RG_AVALID  BIT(2)
+#define P2D_RG_IDDIG   BIT(1)
+
+#define SSPXTP_PHYA_GLB_00 ((SSPXTP_SIFSLV_PHYA_GLB) + 0x00)
+#define RG_XTP_GLB_BIAS_INTR_CTRL  GENMASK(21, 16)
+#define RG_XTP_GLB_BIAS_INTR_CTRL_VAL(x)   ((0x3f & (x)) << 16)
+
+#define SSPXTP_PHYA_LN_04  ((SSPXTP_SIFSLV_PHYA_LN) + 0x04)
+#define RG_XTP_LN0_TX_IMPSEL   GENMASK(4, 0)
+#define RG_XTP_LN0_TX_IMPSEL_VAL(x)(0x1f & (x))
+
+#define SSPXTP_PHYA_LN_14  ((SSPXTP_SIFSLV_PHYA_LN) + 0x014)
+#define RG_XTP_LN0_RX_IMPSEL   GENMASK(4, 0)
+#define RG_XTP_LN0_RX_IMPSEL_VAL(x)(0x1f & (x))
+
+#define XSP_REF_CLK26  /* MHZ */
+#define XSP_SLEW_RATE_COEF 17
+#define XSP_SR_COEF_DIVISOR1000
+#define XSP_FM_DET_CYCLE_CNT   1024
+
+struct xsphy_instance {
+   struct phy *phy;
+   void __iomem *port_base;
+   struct clk *ref_clk;/* reference clock of anolog phy */
+   u32 index;
+   u32 type;
+   /* only for HQA test */
+   int 

[PATCH v2 2/2] phy: mediatek: add XS-PHY driver

[Chunfeng Yun]

Support XS-PHY for MediaTek SoCs with USB3.1 GEN2 controller

Signed-off-by: Chunfeng Yun 
---
 drivers/phy/mediatek/Kconfig |9 +
 drivers/phy/mediatek/Makefile|1 +
 drivers/phy/mediatek/phy-mtk-xsphy.c |  600 ++
 3 files changed, 610 insertions(+)
 create mode 100644 drivers/phy/mediatek/phy-mtk-xsphy.c

diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 88ab4e2..8857d00 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -12,3 +12,12 @@ config PHY_MTK_TPHY
  different banks layout, the T-PHY with shared banks between
  multi-ports is first version, otherwise is second veriosn,
  so you can easily distinguish them by banks layout.
+
+config PHY_MTK_XSPHY
+tristate "MediaTek XS-PHY Driver"
+depends on ARCH_MEDIATEK && OF
+select GENERIC_PHY
+help
+ Enable this to support the SuperSpeedPlus XS-PHY transceiver for
+ USB3.1 GEN2 controllers on MediaTek chips. The driver supports
+ multiple USB2.0, USB3.1 GEN2 ports.
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index 1bdab14..ee49edc 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -4,3 +4,4 @@
 #
 
 obj-$(CONFIG_PHY_MTK_TPHY) += phy-mtk-tphy.o
+obj-$(CONFIG_PHY_MTK_XSPHY)+= phy-mtk-xsphy.o
diff --git a/drivers/phy/mediatek/phy-mtk-xsphy.c 
b/drivers/phy/mediatek/phy-mtk-xsphy.c
new file mode 100644
index 000..020cd02
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-xsphy.c
@@ -0,0 +1,600 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MediaTek USB3.1 gen2 xsphy Driver
+ *
+ * Copyright (c) 2018 MediaTek Inc.
+ * Author: Chunfeng Yun 
+ *
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* u2 phy banks */
+#define SSUSB_SIFSLV_MISC  0x000
+#define SSUSB_SIFSLV_U2FREQ0x100
+#define SSUSB_SIFSLV_U2PHY_COM 0x300
+
+/* u3 phy shared banks */
+#define SSPXTP_SIFSLV_DIG_GLB  0x000
+#define SSPXTP_SIFSLV_PHYA_GLB 0x100
+
+/* u3 phy banks */
+#define SSPXTP_SIFSLV_DIG_LN_TOP   0x000
+#define SSPXTP_SIFSLV_DIG_LN_TX0   0x100
+#define SSPXTP_SIFSLV_DIG_LN_RX0   0x200
+#define SSPXTP_SIFSLV_DIG_LN_DAIF  0x300
+#define SSPXTP_SIFSLV_PHYA_LN  0x400
+
+#define XSP_U2FREQ_FMCR0   ((SSUSB_SIFSLV_U2FREQ) + 0x00)
+#define P2F_RG_FREQDET_EN  BIT(24)
+#define P2F_RG_CYCLECNTGENMASK(23, 0)
+#define P2F_RG_CYCLECNT_VAL(x) ((P2F_RG_CYCLECNT) & (x))
+
+#define XSP_U2FREQ_MMONR0  ((SSUSB_SIFSLV_U2FREQ) + 0x0c)
+
+#define XSP_U2FREQ_FMMONR1 ((SSUSB_SIFSLV_U2FREQ) + 0x10)
+#define P2F_RG_FRCK_EN BIT(8)
+#define P2F_USB_FM_VALID   BIT(0)
+
+#define XSP_USBPHYACR0 ((SSUSB_SIFSLV_U2PHY_COM) + 0x00)
+#define P2A0_RG_INTR_ENBIT(5)
+
+#define XSP_USBPHYACR1 ((SSUSB_SIFSLV_U2PHY_COM) + 0x04)
+#define P2A1_RG_INTR_CAL   GENMASK(23, 19)
+#define P2A1_RG_INTR_CAL_VAL(x)((0x1f & (x)) << 19)
+#define P2A1_RG_VRT_SELGENMASK(14, 12)
+#define P2A1_RG_VRT_SEL_VAL(x) ((0x7 & (x)) << 12)
+#define P2A1_RG_TERM_SEL   GENMASK(10, 8)
+#define P2A1_RG_TERM_SEL_VAL(x)((0x7 & (x)) << 8)
+
+#define XSP_USBPHYACR5 ((SSUSB_SIFSLV_U2PHY_COM) + 0x014)
+#define P2A5_RG_HSTX_SRCAL_EN  BIT(15)
+#define P2A5_RG_HSTX_SRCTRLGENMASK(14, 12)
+#define P2A5_RG_HSTX_SRCTRL_VAL(x) ((0x7 & (x)) << 12)
+
+#define XSP_USBPHYACR6 ((SSUSB_SIFSLV_U2PHY_COM) + 0x018)
+#define P2A6_RG_BC11_SW_EN BIT(23)
+#define P2A6_RG_OTG_VBUSCMP_EN BIT(20)
+
+#define XSP_U2PHYDTM1  ((SSUSB_SIFSLV_U2PHY_COM) + 0x06C)
+#define P2D_FORCE_IDDIGBIT(9)
+#define P2D_RG_VBUSVALID   BIT(5)
+#define P2D_RG_SESSEND BIT(4)
+#define P2D_RG_AVALID  BIT(2)
+#define P2D_RG_IDDIG   BIT(1)
+
+#define SSPXTP_PHYA_GLB_00 ((SSPXTP_SIFSLV_PHYA_GLB) + 0x00)
+#define RG_XTP_GLB_BIAS_INTR_CTRL  GENMASK(21, 16)
+#define RG_XTP_GLB_BIAS_INTR_CTRL_VAL(x)   ((0x3f & (x)) << 16)
+
+#define SSPXTP_PHYA_LN_04  ((SSPXTP_SIFSLV_PHYA_LN) + 0x04)
+#define RG_XTP_LN0_TX_IMPSEL   GENMASK(4, 0)
+#define RG_XTP_LN0_TX_IMPSEL_VAL(x)(0x1f & (x))
+
+#define SSPXTP_PHYA_LN_14  ((SSPXTP_SIFSLV_PHYA_LN) + 0x014)
+#define RG_XTP_LN0_RX_IMPSEL   GENMASK(4, 0)
+#define RG_XTP_LN0_RX_IMPSEL_VAL(x)(0x1f & (x))
+
+#define XSP_REF_CLK26  /* MHZ */
+#define XSP_SLEW_RATE_COEF 17
+#define XSP_SR_COEF_DIVISOR1000
+#define XSP_FM_DET_CYCLE_CNT   1024
+
+struct xsphy_instance {
+   struct phy *phy;
+   void __iomem *port_base;
+   struct clk *ref_clk;/* reference clock of anolog phy */
+   u32 index;
+   u32 type;
+   /* only for HQA test */
+   int efuse_intr;
+   int efuse_tx_imp;
+   int 

[PATCH v2 1/2] dt-bindings: add MediaTek XS-PHY binding

[Chunfeng Yun]

Add a DT binding documentation of XS-PHY for MediaTek SoCs
with USB3.1 GEN2 controller

Signed-off-by: Chunfeng Yun 
---
 .../devicetree/bindings/phy/phy-mtk-xsphy.txt  |  110 
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt 
b/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
new file mode 100644
index 000..9a95fab
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
@@ -0,0 +1,110 @@
+MediaTek XS-PHY binding
+--
+
+The XS-PHY controller supports physical layer functionality for USB3.1
+GEN2 controller on MediaTek SoCs.
+
+Required properties (controller (parent) node):
+ - compatible  : should be "mediatek,-xsphy", "mediatek,xsphy",
+ soc-model is the name of SoC, such as mt2712 etc;
+ when using "mediatek,xsphy" compatible string, you need SoC 
specific
+ ones in addition, one of:
+ - "mediatek,mt3611-xsphy"
+
+ - #address-cells, #size-cells : should use the same values as the root node
+ - ranges: must be present
+
+Optional properties (controller (parent) node):
+ - reg : offset and length of register shared by multiple U3 ports,
+ exclude port's private register, if only U2 ports provided,
+ shouldn't use the property.
+ - mediatek,src-ref-clk-mhz: u32, frequency of reference clock for slew 
rate
+ calibrate
+ - mediatek,src-coef   : u32, coefficient for slew rate calibrate, depends on
+ SoC process
+
+Required nodes : a sub-node is required for each port the controller
+ provides. Address range information including the usual
+ 'reg' property is used inside these nodes to describe
+ the controller's topology.
+
+Required properties (port (child) node):
+- reg  : address and length of the register set for the port.
+- clocks   : a list of phandle + clock-specifier pairs, one for each
+ entry in clock-names
+- clock-names  : must contain
+ "ref": 48M reference clock for HighSpeed analog phy; and 26M
+   reference clock for SuperSpeedPlus analog phy, 
sometimes is
+   24M, 25M or 27M, depended on platform.
+- #phy-cells   : should be 1
+ cell after port phandle is phy type from:
+   - PHY_TYPE_USB2
+   - PHY_TYPE_USB3
+
+The following optional properties are only for debug or HQA test
+Optional properties (PHY_TYPE_USB2 port (child) node):
+- mediatek,eye-src : u32, the value of slew rate calibrate
+- mediatek,eye-vrt : u32, the selection of VRT reference voltage
+- mediatek,eye-term: u32, the selection of HS_TX TERM reference voltage
+- mediatek,efuse-intr  : u32, the selection of Internal Resistor
+
+Optional properties (PHY_TYPE_USB3 port (child) node):
+- mediatek,efuse-intr  : u32, the selection of Internal Resistor
+- mediatek,efuse-tx-imp: u32, the selection of TX Impedance
+- mediatek,efuse-rx-imp: u32, the selection of RX Impedance
+
+Banks layout of xsphy
+-
+portoffsetbank
+u2 port00xMISC
+0x0100FMREG
+0x0300U2PHY_COM
+u2 port10x1000MISC
+0x1100FMREG
+0x1300U2PHY_COM
+u2 port20x2000MISC
+...
+u31 common  0x3000DIG_GLB
+0x3100PHYA_GLB
+u31 port0   0x3400DIG_LN_TOP
+0x3500DIG_LN_TX0
+0x3600DIG_LN_RX0
+0x3700DIG_LN_DAIF
+0x3800PHYA_LN
+u31 port1   0x3a00DIG_LN_TOP
+0x3b00DIG_LN_TX0
+0x3c00DIG_LN_RX0
+0x3d00DIG_LN_DAIF
+0x3e00PHYA_LN
+...
+
+DIG_GLB & PHYA_GLB are shared by U31 ports.
+
+Example:
+
+u3phy: usb-phy@11c4 {
+   compatible = "mediatek,mt3611-xsphy", "mediatek,xsphy";
+   reg = <0 0x11c43000 0 0x0200>;
+   mediatek,src-ref-clk-mhz = <26>;
+   mediatek,src-coef = <17>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   u2port0: usb-phy@11c4 {
+   reg = <0 0x11c4 0 0x0400>;
+   clocks = <>;
+   clock-names = "ref";
+   mediatek,eye-src = <4>;
+   #phy-cells = <1>;
+   };
+
+   u3port0: usb-phy@11c43000 {
+   reg = <0 0x11c43400 0 0x0500>;
+   clocks = <>;
+   clock-names = "ref";
+   mediatek,efuse-intr = <28>;
+   #phy-cells = <1>;
+   };
+};
+
-- 
1.7.9.5

[PATCH v2 1/2] dt-bindings: add MediaTek XS-PHY binding

[Chunfeng Yun]

Add a DT binding documentation of XS-PHY for MediaTek SoCs
with USB3.1 GEN2 controller

Signed-off-by: Chunfeng Yun 
---
 .../devicetree/bindings/phy/phy-mtk-xsphy.txt  |  110 
 1 file changed, 110 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt

diff --git a/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt 
b/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
new file mode 100644
index 000..9a95fab
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
@@ -0,0 +1,110 @@
+MediaTek XS-PHY binding
+--
+
+The XS-PHY controller supports physical layer functionality for USB3.1
+GEN2 controller on MediaTek SoCs.
+
+Required properties (controller (parent) node):
+ - compatible  : should be "mediatek,-xsphy", "mediatek,xsphy",
+ soc-model is the name of SoC, such as mt2712 etc;
+ when using "mediatek,xsphy" compatible string, you need SoC 
specific
+ ones in addition, one of:
+ - "mediatek,mt3611-xsphy"
+
+ - #address-cells, #size-cells : should use the same values as the root node
+ - ranges: must be present
+
+Optional properties (controller (parent) node):
+ - reg : offset and length of register shared by multiple U3 ports,
+ exclude port's private register, if only U2 ports provided,
+ shouldn't use the property.
+ - mediatek,src-ref-clk-mhz: u32, frequency of reference clock for slew 
rate
+ calibrate
+ - mediatek,src-coef   : u32, coefficient for slew rate calibrate, depends on
+ SoC process
+
+Required nodes : a sub-node is required for each port the controller
+ provides. Address range information including the usual
+ 'reg' property is used inside these nodes to describe
+ the controller's topology.
+
+Required properties (port (child) node):
+- reg  : address and length of the register set for the port.
+- clocks   : a list of phandle + clock-specifier pairs, one for each
+ entry in clock-names
+- clock-names  : must contain
+ "ref": 48M reference clock for HighSpeed analog phy; and 26M
+   reference clock for SuperSpeedPlus analog phy, 
sometimes is
+   24M, 25M or 27M, depended on platform.
+- #phy-cells   : should be 1
+ cell after port phandle is phy type from:
+   - PHY_TYPE_USB2
+   - PHY_TYPE_USB3
+
+The following optional properties are only for debug or HQA test
+Optional properties (PHY_TYPE_USB2 port (child) node):
+- mediatek,eye-src : u32, the value of slew rate calibrate
+- mediatek,eye-vrt : u32, the selection of VRT reference voltage
+- mediatek,eye-term: u32, the selection of HS_TX TERM reference voltage
+- mediatek,efuse-intr  : u32, the selection of Internal Resistor
+
+Optional properties (PHY_TYPE_USB3 port (child) node):
+- mediatek,efuse-intr  : u32, the selection of Internal Resistor
+- mediatek,efuse-tx-imp: u32, the selection of TX Impedance
+- mediatek,efuse-rx-imp: u32, the selection of RX Impedance
+
+Banks layout of xsphy
+-
+portoffsetbank
+u2 port00xMISC
+0x0100FMREG
+0x0300U2PHY_COM
+u2 port10x1000MISC
+0x1100FMREG
+0x1300U2PHY_COM
+u2 port20x2000MISC
+...
+u31 common  0x3000DIG_GLB
+0x3100PHYA_GLB
+u31 port0   0x3400DIG_LN_TOP
+0x3500DIG_LN_TX0
+0x3600DIG_LN_RX0
+0x3700DIG_LN_DAIF
+0x3800PHYA_LN
+u31 port1   0x3a00DIG_LN_TOP
+0x3b00DIG_LN_TX0
+0x3c00DIG_LN_RX0
+0x3d00DIG_LN_DAIF
+0x3e00PHYA_LN
+...
+
+DIG_GLB & PHYA_GLB are shared by U31 ports.
+
+Example:
+
+u3phy: usb-phy@11c4 {
+   compatible = "mediatek,mt3611-xsphy", "mediatek,xsphy";
+   reg = <0 0x11c43000 0 0x0200>;
+   mediatek,src-ref-clk-mhz = <26>;
+   mediatek,src-coef = <17>;
+   #address-cells = <2>;
+   #size-cells = <2>;
+   ranges;
+
+   u2port0: usb-phy@11c4 {
+   reg = <0 0x11c4 0 0x0400>;
+   clocks = <>;
+   clock-names = "ref";
+   mediatek,eye-src = <4>;
+   #phy-cells = <1>;
+   };
+
+   u3port0: usb-phy@11c43000 {
+   reg = <0 0x11c43400 0 0x0500>;
+   clocks = <>;
+   clock-names = "ref";
+   mediatek,efuse-intr = <28>;
+   #phy-cells = <1>;
+   };
+};
+
-- 
1.7.9.5

[PATCH v2 0/2] Add MediaTek XS-PHY driver

[Chunfeng Yun]

>From a0814ad7725587a06d273997e0fdf5161f916fd8 Mon Sep 17 00:00:00 2001
From: Chunfeng Yun 
Date: Sat, 5 May 2018 09:56:59 +0800
Subject: [PATCH v2 0/2] Add MediaTek XS-PHY driver

This patch series support the SuperSpeedPlus XS-PHY transceiver for
USB3.1 GEN2 controller on MediaTek chips. The driver supports multiple
USB2.0, USB3.1 GEN2 ports.

v2: changes in binding (suggested by Rob)
1. list all valid SoCs for compatible
2. move required child nodes after parent optional ones
3. remove status property in example
4. move banks layout example before dts one
5. remove phy binding example
6. add #address-cells, #size-cells, ranges properties for parent node

Chunfeng Yun (2):
  dt-bindings: add MediaTek XS-PHY binding
  phy: mediatek: add XS-PHY driver

 .../devicetree/bindings/phy/phy-mtk-xsphy.txt  | 110 
 drivers/phy/mediatek/Kconfig   |   9 +
 drivers/phy/mediatek/Makefile  |   1 +
 drivers/phy/mediatek/phy-mtk-xsphy.c   | 600 +
 4 files changed, 720 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
 create mode 100644 drivers/phy/mediatek/phy-mtk-xsphy.c

-- 
1.9.1

[PATCH v2 0/2] Add MediaTek XS-PHY driver

[Chunfeng Yun]

>From a0814ad7725587a06d273997e0fdf5161f916fd8 Mon Sep 17 00:00:00 2001
From: Chunfeng Yun 
Date: Sat, 5 May 2018 09:56:59 +0800
Subject: [PATCH v2 0/2] Add MediaTek XS-PHY driver

This patch series support the SuperSpeedPlus XS-PHY transceiver for
USB3.1 GEN2 controller on MediaTek chips. The driver supports multiple
USB2.0, USB3.1 GEN2 ports.

v2: changes in binding (suggested by Rob)
1. list all valid SoCs for compatible
2. move required child nodes after parent optional ones
3. remove status property in example
4. move banks layout example before dts one
5. remove phy binding example
6. add #address-cells, #size-cells, ranges properties for parent node

Chunfeng Yun (2):
  dt-bindings: add MediaTek XS-PHY binding
  phy: mediatek: add XS-PHY driver

 .../devicetree/bindings/phy/phy-mtk-xsphy.txt  | 110 
 drivers/phy/mediatek/Kconfig   |   9 +
 drivers/phy/mediatek/Makefile  |   1 +
 drivers/phy/mediatek/phy-mtk-xsphy.c   | 600 +
 4 files changed, 720 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/phy-mtk-xsphy.txt
 create mode 100644 drivers/phy/mediatek/phy-mtk-xsphy.c

-- 
1.9.1

RE: [External] Re: [PATCH 3/3] mm/page_alloc: Fix typo in debug info of calculate_node_totalpages

[Huaisheng HS1 Ye]

> On Fri 04-05-18 14:52:09, Huaisheng Ye wrote:
> > realtotalpages is calculated by taking off absent_pages from
> > spanned_pages in every zone.
> > Debug message of calculate_node_totalpages shall accurately
> > indicate that it is real totalpages to avoid ambiguity.
> 
> Is the printk actually useful? Why don't we simply remove it? You can
> get the information from /proc/zoneinfo so why to litter the dmesg
> output?

Indeed, we can get the amount of pfns as spanned, present and managed
from /proc/zoneinfo after memory initialization has been finished.

But this printk is a relatively meaningful reference within dmesg log.
Especially for people who doesn't have much experience, or someone
has a plan to modify boundary of zones within free_area_init_*.

Sincerely,
Huaisheng Ye
Linux kernel | Lenovo
> 
> > Signed-off-by: Huaisheng Ye 
> > ---
> >  mm/page_alloc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 1b39db4..9d57db2 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -5967,7 +5967,7 @@ static void __meminit
> calculate_node_totalpages(struct pglist_data *pgdat,
> >
> > pgdat->node_spanned_pages = totalpages;
> > pgdat->node_present_pages = realtotalpages;
> > -   printk(KERN_DEBUG "On node %d totalpages: %lu\n", pgdat->node_id,
> > +   printk(KERN_DEBUG "On node %d realtotalpages: %lu\n",
> pgdat->node_id,
> > realtotalpages);
> >  }
> >
> > --
> > 1.8.3.1

RE: [External] Re: [PATCH 3/3] mm/page_alloc: Fix typo in debug info of calculate_node_totalpages

[Huaisheng HS1 Ye]

> On Fri 04-05-18 14:52:09, Huaisheng Ye wrote:
> > realtotalpages is calculated by taking off absent_pages from
> > spanned_pages in every zone.
> > Debug message of calculate_node_totalpages shall accurately
> > indicate that it is real totalpages to avoid ambiguity.
> 
> Is the printk actually useful? Why don't we simply remove it? You can
> get the information from /proc/zoneinfo so why to litter the dmesg
> output?

Indeed, we can get the amount of pfns as spanned, present and managed
from /proc/zoneinfo after memory initialization has been finished.

But this printk is a relatively meaningful reference within dmesg log.
Especially for people who doesn't have much experience, or someone
has a plan to modify boundary of zones within free_area_init_*.

Sincerely,
Huaisheng Ye
Linux kernel | Lenovo
> 
> > Signed-off-by: Huaisheng Ye 
> > ---
> >  mm/page_alloc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 1b39db4..9d57db2 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -5967,7 +5967,7 @@ static void __meminit
> calculate_node_totalpages(struct pglist_data *pgdat,
> >
> > pgdat->node_spanned_pages = totalpages;
> > pgdat->node_present_pages = realtotalpages;
> > -   printk(KERN_DEBUG "On node %d totalpages: %lu\n", pgdat->node_id,
> > +   printk(KERN_DEBUG "On node %d realtotalpages: %lu\n",
> pgdat->node_id,
> > realtotalpages);
> >  }
> >
> > --
> > 1.8.3.1

[GIT PULL] Kbuild fixes for 4.17-rc4

[Masahiro Yamada]

Hi Linus,

Please pull some Kbuild fixes.
Thanks!


The following changes since commit 6da6c0db5316275015e8cc2959f12a17584aeb64:

  Linux v4.17-rc3 (2018-04-29 14:17:42 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
tags/kbuild-fixes-v4.17

for you to fetch changes up to 0da7e43261142b93307b70da455376ad84414d0a:

  genksyms: fix typo in parse.tab.{c,h} generation rules (2018-05-05
10:24:53 +0900)


Kbuild fixes for v4.17

- remove state comment in modpost

- extend MAINTAINERS entry to cover modpost and more makefiles

- fix missed building of SANCOV gcc-plugin

- replace left-over 'bison' with $(YACC)

- display short log when generating parer of genksyms


Masahiro Yamada (2):
  gcc-plugins: fix build condition of SANCOV plugin
  kbuild: replace hardcoded bison in cmd_bison_h with $(YACC)

Mauro Rossi (1):
  genksyms: fix typo in parse.tab.{c,h} generation rules

Rasmus Villemoes (2):
  modpost: delete stale comment
  MAINTAINERS: Update Kbuild entry with a few paths

 MAINTAINERS  | 4 +++-
 scripts/Makefile.gcc-plugins | 2 +-
 scripts/Makefile.lib | 2 +-
 scripts/genksyms/Makefile| 4 ++--
 scripts/mod/sumversion.c | 9 +
 5 files changed, 8 insertions(+), 13 deletions(-)






-- 
Best Regards
Masahiro Yamada

[GIT PULL] Kbuild fixes for 4.17-rc4

[Masahiro Yamada]

Hi Linus,

Please pull some Kbuild fixes.
Thanks!


The following changes since commit 6da6c0db5316275015e8cc2959f12a17584aeb64:

  Linux v4.17-rc3 (2018-04-29 14:17:42 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild.git
tags/kbuild-fixes-v4.17

for you to fetch changes up to 0da7e43261142b93307b70da455376ad84414d0a:

  genksyms: fix typo in parse.tab.{c,h} generation rules (2018-05-05
10:24:53 +0900)


Kbuild fixes for v4.17

- remove state comment in modpost

- extend MAINTAINERS entry to cover modpost and more makefiles

- fix missed building of SANCOV gcc-plugin

- replace left-over 'bison' with $(YACC)

- display short log when generating parer of genksyms


Masahiro Yamada (2):
  gcc-plugins: fix build condition of SANCOV plugin
  kbuild: replace hardcoded bison in cmd_bison_h with $(YACC)

Mauro Rossi (1):
  genksyms: fix typo in parse.tab.{c,h} generation rules

Rasmus Villemoes (2):
  modpost: delete stale comment
  MAINTAINERS: Update Kbuild entry with a few paths

 MAINTAINERS  | 4 +++-
 scripts/Makefile.gcc-plugins | 2 +-
 scripts/Makefile.lib | 2 +-
 scripts/genksyms/Makefile| 4 ++--
 scripts/mod/sumversion.c | 9 +
 5 files changed, 8 insertions(+), 13 deletions(-)






-- 
Best Regards
Masahiro Yamada

Re: Oops on the system startup in the function part_in_flight()

[Jens Axboe]

On 5/4/18 6:35 PM, Ben Greear wrote:
> Hello,
> 
> I am trying to bisect a pktgen related performance regression that appears to
> be between the 4.13 and 4.14 kernels.  But, my system keeps crashing due to 
> part_in_flight
> oops so bisecting is not going well.
> 
> It looks like this oops was fixed, but the link to the patch in the email is 
> no longer
> valid.  Can someone let me know what patch fixes this crash so I can apply it 
> while
> bisecting?
> 
> https://www.spinics.net/lists/linux-block/msg17809.html

Should be this one:

http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus=f5c156c4c29a3d87176dd6e5c099388e187ec29b

-- 
Jens Axboe

Re: Oops on the system startup in the function part_in_flight()

[Jens Axboe]

On 5/4/18 6:35 PM, Ben Greear wrote:
> Hello,
> 
> I am trying to bisect a pktgen related performance regression that appears to
> be between the 4.13 and 4.14 kernels.  But, my system keeps crashing due to 
> part_in_flight
> oops so bisecting is not going well.
> 
> It looks like this oops was fixed, but the link to the patch in the email is 
> no longer
> valid.  Can someone let me know what patch fixes this crash so I can apply it 
> while
> bisecting?
> 
> https://www.spinics.net/lists/linux-block/msg17809.html

Should be this one:

http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus=f5c156c4c29a3d87176dd6e5c099388e187ec29b

-- 
Jens Axboe

[GIT PULL] clk fixes for v4.17-rc3

[sboyd]

From: Stephen Boyd 

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
tags/clk-fixes-for-linus

for you to fetch changes up to c964cfc612b59910593fa10ee1c2673db274c9c7:

  Merge tag 'meson-clk-fixes-4.17-1' of https://github.com/BayLibre/clk-meson 
into clk-fixes (2018-05-01 14:44:16 -0700)


A handful of fixes for the stm32mp1 clk driver came in during the merge
window for the driver that got merged in the merge window. Plus a
warning fix for unused PM ops and a couple fixes for the meson clk
driver clk names that went unnoticed with the regmap rework. There's
also another fix in here for the mux rounding flag which wasn't doing
what it said it did, but now it does.


Arnd Bergmann (1):
  clk: cs2000: mark resume function as __maybe_unused

Gabriel Fernandez (6):
  clk: stm32mp1: add missing static
  clk: stm32mp1: remove unused dfsdm_src[] const
  clk: stm32mp1: fix SAI3 & SAI4 clocks
  clk: stm32mp1: add missing tzc2 clock
  clk: stm32mp1: set stgen_k clock as critical
  clk: stm32mp1: remove ck_apb_dbg clock

Jerome Brunet (2):
  clk: honor CLK_MUX_ROUND_CLOSEST in generic clk mux
  clk: meson: honor CLK_MUX_ROUND_CLOSEST in clk_regmap

Martin Blumenstingl (2):
  clk: meson: meson8b: fix meson8b_fclk_div3_div clock name
  clk: meson: meson8b: fix meson8b_cpu_clk parent clock name

Stephen Boyd (2):
  Merge branch 'clk-stm32mp1' into clk-fixes
  Merge tag 'meson-clk-fixes-4.17-1' of 
https://github.com/BayLibre/clk-meson into clk-fixes

Yixun Lan (1):
  clk: meson: drop meson_aoclk_gate_regmap_ops

 drivers/clk/clk-cs2000-cp.c   |  2 +-
 drivers/clk/clk-mux.c | 10 +-
 drivers/clk/clk-stm32mp1.c| 54 +--
 drivers/clk/clk.c |  7 ++--
 drivers/clk/meson/clk-regmap.c| 11 ++-
 drivers/clk/meson/gxbb-aoclk.h|  2 --
 drivers/clk/meson/meson8b.c   |  5 +--
 include/dt-bindings/clock/stm32mp1-clks.h |  4 +--
 include/linux/clk-provider.h  |  3 ++
 9 files changed, 55 insertions(+), 43 deletions(-)

-- 
Sent by a computer through tubes

[GIT PULL] clk fixes for v4.17-rc3

[sboyd]

From: Stephen Boyd 

The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338:

  Linux 4.17-rc1 (2018-04-15 18:24:20 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git 
tags/clk-fixes-for-linus

for you to fetch changes up to c964cfc612b59910593fa10ee1c2673db274c9c7:

  Merge tag 'meson-clk-fixes-4.17-1' of https://github.com/BayLibre/clk-meson 
into clk-fixes (2018-05-01 14:44:16 -0700)


A handful of fixes for the stm32mp1 clk driver came in during the merge
window for the driver that got merged in the merge window. Plus a
warning fix for unused PM ops and a couple fixes for the meson clk
driver clk names that went unnoticed with the regmap rework. There's
also another fix in here for the mux rounding flag which wasn't doing
what it said it did, but now it does.


Arnd Bergmann (1):
  clk: cs2000: mark resume function as __maybe_unused

Gabriel Fernandez (6):
  clk: stm32mp1: add missing static
  clk: stm32mp1: remove unused dfsdm_src[] const
  clk: stm32mp1: fix SAI3 & SAI4 clocks
  clk: stm32mp1: add missing tzc2 clock
  clk: stm32mp1: set stgen_k clock as critical
  clk: stm32mp1: remove ck_apb_dbg clock

Jerome Brunet (2):
  clk: honor CLK_MUX_ROUND_CLOSEST in generic clk mux
  clk: meson: honor CLK_MUX_ROUND_CLOSEST in clk_regmap

Martin Blumenstingl (2):
  clk: meson: meson8b: fix meson8b_fclk_div3_div clock name
  clk: meson: meson8b: fix meson8b_cpu_clk parent clock name

Stephen Boyd (2):
  Merge branch 'clk-stm32mp1' into clk-fixes
  Merge tag 'meson-clk-fixes-4.17-1' of 
https://github.com/BayLibre/clk-meson into clk-fixes

Yixun Lan (1):
  clk: meson: drop meson_aoclk_gate_regmap_ops

 drivers/clk/clk-cs2000-cp.c   |  2 +-
 drivers/clk/clk-mux.c | 10 +-
 drivers/clk/clk-stm32mp1.c| 54 +--
 drivers/clk/clk.c |  7 ++--
 drivers/clk/meson/clk-regmap.c| 11 ++-
 drivers/clk/meson/gxbb-aoclk.h|  2 --
 drivers/clk/meson/meson8b.c   |  5 +--
 include/dt-bindings/clock/stm32mp1-clks.h |  4 +--
 include/linux/clk-provider.h  |  3 ++
 9 files changed, 55 insertions(+), 43 deletions(-)

-- 
Sent by a computer through tubes

Re: I2C PM overhaul needed? (Re: [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called)

[Mark Brown]

On Fri, May 04, 2018 at 02:24:47PM +0200, Wolfram Sang wrote:

> To handle that, I imagined an additional adapter callback like
> 'master_xfer_irqless' to be used for such special I2C messages. These
> kind of special messages could be tagged with a new I2C_M_something
> flag.

> And maybe this could be used here, too? Introduce this flag for very
> late/early messages. If they have it, messages are even sent in
> suspend_noirq() phase with the master_xfer_irqless() callback, otherwise
> we will have the WARNing printed out.

It feels like it'd be more elegant to have the core select the irqless
function automatically if called after interrupts have been disabled -
otherwise we end up with the need to special case through other layers
of the stack like regmap as well which seems like it'd be error prone.
OTOH it does mean we might not notice things happening later than they
should so it's not 100% clear...


signature.asc
Description: PGP signature

Re: I2C PM overhaul needed? (Re: [PATCH 1/2] i2c: sprd: Prevent i2c accesses after suspend is called)

[Mark Brown]

On Fri, May 04, 2018 at 02:24:47PM +0200, Wolfram Sang wrote:

> To handle that, I imagined an additional adapter callback like
> 'master_xfer_irqless' to be used for such special I2C messages. These
> kind of special messages could be tagged with a new I2C_M_something
> flag.

> And maybe this could be used here, too? Introduce this flag for very
> late/early messages. If they have it, messages are even sent in
> suspend_noirq() phase with the master_xfer_irqless() callback, otherwise
> we will have the WARNing printed out.

It feels like it'd be more elegant to have the core select the irqless
function automatically if called after interrupts have been disabled -
otherwise we end up with the need to special case through other layers
of the stack like regmap as well which seems like it'd be error prone.
OTOH it does mean we might not notice things happening later than they
should so it's not 100% clear...


signature.asc
Description: PGP signature

Re: [PATCH 21/24] selftests: memfd: return Kselftest Skip code for skipped tests

[Mike Kravetz]

On 05/04/2018 06:13 PM, Shuah Khan (Samsung OSG) wrote:
> When memfd test is skipped because of unmet dependencies and/or unsupported
> configuration, it returns non-zero value which is treated as a fail by the
> Kselftest framework. This leads to false negative result even when the test
> could not be run.
> 
> Change it to return kselftest skip code when a test gets skipped to
> clearly report that the test could not be run.
> 
> Added an explicit check for root user and return skip code.
> 
> Kselftest framework SKIP code is 4 and the framework prints appropriate
> messages to indicate that the test is skipped.
> 
> Signed-off-by: Shuah Khan (Samsung OSG) 
> ---
>  tools/testing/selftests/memfd/run_tests.sh | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/memfd/run_tests.sh 
> b/tools/testing/selftests/memfd/run_tests.sh
> index c2d41ed81b24..88dc206a69b7 100755
> --- a/tools/testing/selftests/memfd/run_tests.sh
> +++ b/tools/testing/selftests/memfd/run_tests.sh
> @@ -1,6 +1,14 @@
>  #!/bin/bash
>  # please run as root
>  
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +if [ $UID != 0 ]; then
> + echo "Please run this test as root"
> + exit $ksft_skip
> +fi
> +
>  #
>  # Normal tests requiring no special resources
>  #
> @@ -33,7 +41,7 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then
>   echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages
>   if [ $? -ne 0 ]; then
>   echo "Please run this test as root"
> - exit 1
> + exit $ksft_skip

We now KNOW that we are running as root because of the check above.  We
can delete this test, and rely on the later check to determine if the
number of huge pages was actually increased.

How about this instead (untested)?

Signed-off-by: Mike Kravetz 
diff --git a/tools/testing/selftests/memfd/run_tests.sh 
b/tools/testing/selftests/memfd/run_tests.sh
index c2d41ed81b24..99a265a84e1d 100755
--- a/tools/testing/selftests/memfd/run_tests.sh
+++ b/tools/testing/selftests/memfd/run_tests.sh
@@ -1,6 +1,14 @@
 #!/bin/bash
 # please run as root
 
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+if [ $UID != 0 ]; then
+   echo "Please run this test as root"
+   exit $ksft_skip
+fi
+
 #
 # Normal tests requiring no special resources
 #
@@ -31,10 +39,6 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then
 
echo 3 > /proc/sys/vm/drop_caches
echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages
-   if [ $? -ne 0 ]; then
-   echo "Please run this test as root"
-   exit 1
-   fi
while read name size unit; do
if [ "$name" = "HugePages_Free:" ]; then
freepgs=$size
@@ -53,7 +57,7 @@ if [ $freepgs -lt $hpages_test ]; then
fi
printf "Not enough huge pages available (%d < %d)\n" \
$freepgs $needpgs
-   exit 1
+   exit $ksft_skip
 fi
 
 #

Re: [PATCH 21/24] selftests: memfd: return Kselftest Skip code for skipped tests

[Mike Kravetz]

On 05/04/2018 06:13 PM, Shuah Khan (Samsung OSG) wrote:
> When memfd test is skipped because of unmet dependencies and/or unsupported
> configuration, it returns non-zero value which is treated as a fail by the
> Kselftest framework. This leads to false negative result even when the test
> could not be run.
> 
> Change it to return kselftest skip code when a test gets skipped to
> clearly report that the test could not be run.
> 
> Added an explicit check for root user and return skip code.
> 
> Kselftest framework SKIP code is 4 and the framework prints appropriate
> messages to indicate that the test is skipped.
> 
> Signed-off-by: Shuah Khan (Samsung OSG) 
> ---
>  tools/testing/selftests/memfd/run_tests.sh | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/memfd/run_tests.sh 
> b/tools/testing/selftests/memfd/run_tests.sh
> index c2d41ed81b24..88dc206a69b7 100755
> --- a/tools/testing/selftests/memfd/run_tests.sh
> +++ b/tools/testing/selftests/memfd/run_tests.sh
> @@ -1,6 +1,14 @@
>  #!/bin/bash
>  # please run as root
>  
> +# Kselftest framework requirement - SKIP code is 4.
> +ksft_skip=4
> +
> +if [ $UID != 0 ]; then
> + echo "Please run this test as root"
> + exit $ksft_skip
> +fi
> +
>  #
>  # Normal tests requiring no special resources
>  #
> @@ -33,7 +41,7 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then
>   echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages
>   if [ $? -ne 0 ]; then
>   echo "Please run this test as root"
> - exit 1
> + exit $ksft_skip

We now KNOW that we are running as root because of the check above.  We
can delete this test, and rely on the later check to determine if the
number of huge pages was actually increased.

How about this instead (untested)?

Signed-off-by: Mike Kravetz 
diff --git a/tools/testing/selftests/memfd/run_tests.sh 
b/tools/testing/selftests/memfd/run_tests.sh
index c2d41ed81b24..99a265a84e1d 100755
--- a/tools/testing/selftests/memfd/run_tests.sh
+++ b/tools/testing/selftests/memfd/run_tests.sh
@@ -1,6 +1,14 @@
 #!/bin/bash
 # please run as root
 
+# Kselftest framework requirement - SKIP code is 4.
+ksft_skip=4
+
+if [ $UID != 0 ]; then
+   echo "Please run this test as root"
+   exit $ksft_skip
+fi
+
 #
 # Normal tests requiring no special resources
 #
@@ -31,10 +39,6 @@ if [ -n "$freepgs" ] && [ $freepgs -lt $hpages_test ]; then
 
echo 3 > /proc/sys/vm/drop_caches
echo $(( $hpages_needed + $nr_hugepgs )) > /proc/sys/vm/nr_hugepages
-   if [ $? -ne 0 ]; then
-   echo "Please run this test as root"
-   exit 1
-   fi
while read name size unit; do
if [ "$name" = "HugePages_Free:" ]; then
freepgs=$size
@@ -53,7 +57,7 @@ if [ $freepgs -lt $hpages_test ]; then
fi
printf "Not enough huge pages available (%d < %d)\n" \
$freepgs $needpgs
-   exit 1
+   exit $ksft_skip
 fi
 
 #