Re: [PATCH v14 3/8] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls
Hi Maxime, On Thu, 3 May 2018 16:46:19 +0200 Maxime Ripard wrote: > Doesn't that mean that the controls will be shared between the right > and left mixers now, which wasn't the case before? Yes. However Chen-Yu said that except for debugfs that cannot be observed by user space anyway. It's nice to have the NEW controls have multiple channels, which is what this does. >And also, wouldn't > the controls be called "Left Mixer Left Mixer Left DAC Playback > Switch" (for the first one) now? No. I checked using "amixer" - the names look fine, "Left Mixer Left DAC Playback Switch". With the patch series, amixer says: numid=10,iface=MIXER,name='FM Playback Switch' numid=6,iface=MIXER,name='FM Playback Volume' numid=5,iface=MIXER,name='Line Boost Volume' numid=9,iface=MIXER,name='Line Playback Switch' numid=4,iface=MIXER,name='Line Playback Volume' numid=1,iface=MIXER,name='Mic1 Boost Volume' numid=11,iface=MIXER,name='Mic1 Playback Switch' numid=2,iface=MIXER,name='Mic2 Boost Volume' numid=12,iface=MIXER,name='Mic2 Playback Switch' numid=7,iface=MIXER,name='Mic Playback Volume' numid=18,iface=MIXER,name='Capture Source' numid=19,iface=MIXER,name='Differential Line Source' numid=8,iface=MIXER,name='Left Mixer Left DAC Playback Switch' numid=15,iface=MIXER,name='Power Amplifier DAC Playback Switch' numid=16,iface=MIXER,name='Power Amplifier Mixer Playback Switch' numid=17,iface=MIXER,name='Power Amplifier Mute Switch' numid=3,iface=MIXER,name='Power Amplifier Volume' numid=14,iface=MIXER,name='Right Mixer Left DAC Playback Switch' numid=13,iface=MIXER,name='Right Mixer Right DAC Playback Switch' pgpinD_1hMd77.pgp Description: OpenPGP digital signature
[PATCH] scsi: mpt3sas: fix a missing-check bug
In _ctl_ioctl_main(), 'ioctl_header' is first copied from the userspace pointer 'arg'. 'ioctl_header.ioc_number' is then verified by _ctl_verify_adapter(). If the verification is failed, an error code -ENODEV is returned. Otherwise, the verification result, i.e., the MPT3SAS adapter that matches with the 'ioctl_header.ioc_number', is saved to 'ioc'. Later on, if the 'cmd' is MPT3COMMAND, the whole ioctl command struct is copied again from the userspace pointer 'arg' and saved to 'karg'. Then the function _ctl_do_mpt_command() is invoked to execute the command with the adapter 'ioc' and 'karg' as inputs. Given that the pointer 'arg' resides in userspace, a malicious userspace process can race to change the 'ioc_number' between the two copies, which will cause inconsistency issues, potentially security issues since an inconsistent adapter could be used with the command struct 'karg' as inputs of _ctl_do_mpt_command(). Moreover, the user can potentially provide a valid 'ioc_number' to pass the verification, and then modify it to an invalid 'ioc_number'. That means, an invalid 'ioc_number' can potentially bypass the verification check. To fix this issue, we need to recheck the 'ioc_number' copied after the second copy to make sure it is not changed since the first copy. Signed-off-by: Wenwen Wang --- drivers/scsi/mpt3sas/mpt3sas_ctl.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/scsi/mpt3sas/mpt3sas_ctl.c b/drivers/scsi/mpt3sas/mpt3sas_ctl.c index d3cb387..0c140c7 100644 --- a/drivers/scsi/mpt3sas/mpt3sas_ctl.c +++ b/drivers/scsi/mpt3sas/mpt3sas_ctl.c @@ -2388,6 +2388,11 @@ _ctl_ioctl_main(struct file *file, unsigned int cmd, void __user *arg, break; } + if (karg.hdr.ioc_number != ioctl_header.ioc_number) { + ret = -EINVAL; + break; + } + if (_IOC_SIZE(cmd) == sizeof(struct mpt3_ioctl_command)) { uarg = arg; ret = _ctl_do_mpt_command(ioc, karg, &uarg->mf); -- 2.7.4
Re: [PATCH 01/10] crypto: aead - allow to allocate AEAD requests on the stack
On Fri, May 04, 2018 at 09:18:41AM +0200, 'Antoine Tenart' wrote: > > In this driver we need to perform in certain cases an invalidation, > which is done thanks to invalidation requests. To do this we create > dummy requests, using SKCIPHER_REQUEST_ON_STACK and > AHASH_REQUEST_ON_STACK for ciphers and hashes. So when adding the AEAD > algs support, in patch 8/10, AEAD_REQUEST_ON_STACK is used for the same > reason. > > Should we allocate this in a different way? These are not uses intended for the ON_STACK macros. They were only ever meant for existing users of the synchonous crypto API. I would suggest either allocating a new request on the spot or if that is not convenient, pre-allocating it in the cra_init function. Cheers, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] kernel/signal: Remove no longer required irqsave/restore
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
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 = &schan->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(&schan->vc, &sdesc->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"
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, &p); >> >> @@ -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 = &default_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()
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, &fi->inmem_pages, true, false); > } else { > + /* wait all committed IOs writeback and release them from list > */ > __revoke_inmem_pages(inode, &revoke_list, false, false); > } > > -- > 2.15.0.55.gc2ece9dc4de6 >> >> . >> > > > . >
Re: [PATCH 2/2] leds: Add Spreadtrum SC27xx breathing light controller driver
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 = &pdev->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", &base); 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
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
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(). > >> * Unfortun
Re: Suggested new user link command
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
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
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
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
"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
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
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 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->f_pos_lock){+.+.}, at: __run_timers+0x16e/0xc50 kernel/time/timer.c:1658 2 locks held by getty/4452: #0: 3abe4bd2 (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: 35e35fb8 (&ldata->atomic_read_lock){+.+.}, at: n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131 2 locks held by getty/4453: #0: 4e78faf9 (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: 44d079f2 (&ldata->atomic_read_lock){+.+.}, at: n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131 2 locks held by getty/4454: #0: 37bf7fca (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: fc65c2e0 (&ldata->atomic_read_lock){+.+.}, at: n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131 2 locks held by getty/4455: #0: 650b41ff (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: f8a69a89 (&ldata->atomic_read_lock){+.+.}, at: n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131 2 locks held by getty/4456: #0: 33547e18 (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: 0c85318d (&ldata->atomic_read_lock){+.+.}, at: n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131 2 locks held by getty/4457: #0: e5cb3908 (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: 9fc1aed4 (&ldata->atomic_read_lock){+.+.}, at: n_tty_read+0x321/0x1cc0 drivers/tty/n_tty.c:2131 2 locks held by getty/4458: #0: 55360c24 (&tty->ldisc_sem){}, at: ldsem_down_read+0x37/0x40 drivers/tty/tty_ldsem.c:365 #1: 2bcd4fa8 (&ldata->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->lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130 drivers/block/loop.c:1391 1 lock held by syz-executor148/4500: #0: bf14345a (&lo->lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130 drivers/block/loop.c:1391 1 lock held by syz-executor148/4514: #0: bf14345a (&lo->lo_ctl_mutex/1){+.+.}, at: lo_ioctl+0x8d/0x2130 drivers/block/loop.c:1391 1 lock held by syz-
Re: *alloc API changes
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, &c)) > 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
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
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
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
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
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 = &ili9881c_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 = &ili9881c_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
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, &c)) 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
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 *)&ssid, - (struct p80211pstrd *)&wlandev->ssid); + prism2mgmt_bytestr2pstr((struct hfa384x_bytestr *)&ssid, + (struct p80211pstrd *)&wlandev->ssid); /* Collect the port status */ result = hfa384x_drvr_getconfig16(hw, - HFA384x_RID_PORTSTATUS, - &portstatus); + HFA384x_RID_PORTSTATUS, + &portstatus); if (result) { pr_debug ("getconfig(0x%02x) failed, result = %d\n", @@ -1404,7 +1402,7 @@ void prism2sta_processing_defer(struct work_struct *data) &joinreq, HFA384x_RID_JOINREQUEST_LEN); netdev_info(wlandev->netdev, - "linkstatus=ASSOCFAIL (re-submitting join)\n"); + "linkstatus=ASSOCFAIL (re-submitting jo
Re: [PATCH v6 1/3] clk: qcom: Configure the RCGs to a safe source as needed
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, &safe_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
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
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
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
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
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
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
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
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 jp
Re: [v2 PATCH 1/1] tg3: fix meaningless hw_stats reading after tg3_halt memset 0 hw_stats
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(&tp->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(&tp->lock); - if (!tp->hw_stats) { + if (!tp->hw_stats || tg3_flag(tp, INIT_COMPLETE)) { *stats = tp->net_stats_prev; spin_unlock_bh(&tp->lock); return; Cheers, Zumeng
Re: [PATCH 1/2] tpm: do not suspend/resume if power stays on
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
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(&pdev->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
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
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
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
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 = &ssusb->otg_switch; - if (otg_sx->manual_drd_enabled) { + if (otg_sx->manual_drd_enabled) ssusb_debugfs_init(ssusb); - } else { - INIT_DELAYED_WORK(&otg_sx->extcon_reg_dwork, - extcon_register_dwork); - - /* -* It is enough to delay 1s for waiting for -* host initialization -*/ - schedule_delayed_work(&otg_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(&otg_sx->extcon_reg_dwork); } -- 1.9.1
Re: [PATCH v2] f2fs: fix to avoid race during access gc_thread pointer
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 &sb->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 f2
[RESEND PATCH 2/5] usb: mtu3: remove repeated setting of gadget state
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, &mtu->g); - if (ret) { + if (ret) dev_err(mtu->dev, "failed to register udc\n"); - return ret; - } - usb_gadget_set_state(&mtu->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
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 efuse_
[PATCH v2 1/2] dt-bindings: add MediaTek XS-PHY binding
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 = <&clk48m>; + clock-names = "ref"; + mediatek,eye-src = <4>; + #phy-cells = <1>; + }; + + u3port0: usb-phy@11c43000 { + reg = <0 0x11c43400 0 0x0500>; + clocks = <&clk26m>; + clock-names = "ref"; + mediatek,efuse-intr = <28>; + #phy-cells = <1>; + }; +}; + -- 1.7.9.5
[PATCH v2 0/2] Add MediaTek XS-PHY driver
>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
> 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
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()
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&id=f5c156c4c29a3d87176dd6e5c099388e187ec29b -- Jens Axboe
[GIT PULL] clk fixes for v4.17-rc3
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)
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
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 #
Applied "media: i2c: tda1997: replace codec to component" to the asoc tree
The patch media: i2c: tda1997: replace codec to component has been applied to the asoc tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 2d8102cc9a27577ffa4335aaaed4a26060688de2 Mon Sep 17 00:00:00 2001 From: Kuninori Morimoto Date: Mon, 23 Apr 2018 02:10:26 + Subject: [PATCH] media: i2c: tda1997: replace codec to component Now we can replace Codec to Component. Let's do it. Note: xxx_codec_xxx() -> xxx_component_xxx() .idle_bias_off = 0 -> .idle_bias_on = 1 .ignore_pmdown_time = 0 -> .use_pmdown_time = 1 - -> .endianness = 1 - -> .non_legacy_dai_naming = 1 Signed-off-by: Kuninori Morimoto Tested-by: Tim Harvey Acked-by: Tim Harvey Acked-by: Mauro Carvalho Chehab Signed-off-by: Mark Brown --- drivers/media/i2c/tda1997x.c | 25 + 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/drivers/media/i2c/tda1997x.c b/drivers/media/i2c/tda1997x.c index 3021913c28fa..33d7fcf541fc 100644 --- a/drivers/media/i2c/tda1997x.c +++ b/drivers/media/i2c/tda1997x.c @@ -2444,7 +2444,7 @@ static int tda1997x_pcm_startup(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { struct tda1997x_state *state = snd_soc_dai_get_drvdata(dai); - struct snd_soc_codec *codec = dai->codec; + struct snd_soc_component *component = dai->component; struct snd_pcm_runtime *rtd = substream->runtime; int rate, err; @@ -2452,11 +2452,11 @@ static int tda1997x_pcm_startup(struct snd_pcm_substream *substream, err = snd_pcm_hw_constraint_minmax(rtd, SNDRV_PCM_HW_PARAM_RATE, rate, rate); if (err < 0) { - dev_err(codec->dev, "failed to constrain samplerate to %dHz\n", + dev_err(component->dev, "failed to constrain samplerate to %dHz\n", rate); return err; } - dev_info(codec->dev, "set samplerate constraint to %dHz\n", rate); + dev_info(component->dev, "set samplerate constraint to %dHz\n", rate); return 0; } @@ -2479,20 +2479,22 @@ static struct snd_soc_dai_driver tda1997x_audio_dai = { .ops = &tda1997x_dai_ops, }; -static int tda1997x_codec_probe(struct snd_soc_codec *codec) +static int tda1997x_codec_probe(struct snd_soc_component *component) { return 0; } -static int tda1997x_codec_remove(struct snd_soc_codec *codec) +static void tda1997x_codec_remove(struct snd_soc_component *component) { - return 0; } -static struct snd_soc_codec_driver tda1997x_codec_driver = { - .probe = tda1997x_codec_probe, - .remove = tda1997x_codec_remove, - .reg_word_size = sizeof(u16), +static struct snd_soc_component_driver tda1997x_codec_driver = { + .probe = tda1997x_codec_probe, + .remove = tda1997x_codec_remove, + .idle_bias_on = 1, + .use_pmdown_time= 1, + .endianness = 1, + .non_legacy_dai_naming = 1, }; static int tda1997x_probe(struct i2c_client *client, @@ -2737,7 +2739,7 @@ static int tda1997x_probe(struct i2c_client *client, else formats = SNDRV_PCM_FMTBIT_S16_LE; tda1997x_audio_dai.capture.formats = formats; - ret = snd_soc_register_codec(&state->client->dev, + ret = devm_snd_soc_register_component(&state->client->dev, &tda1997x_codec_driver, &tda1997x_audio_dai, 1); if (ret) { @@ -2782,7 +2784,6 @@ static int tda1997x_remove(struct i2c_client *client) struct tda1997x_platform_data *pdata = &state->pdata; if (pdata->audout_format) { - snd_soc_unregister_codec(&client->dev); mutex_destroy(&state->audio_lock); } -- 2.17.0
[PATCH v2] phy: phy-mtk-tphy: use SPDX license tag
Use SPDX-License-Identifier tag instead of the GPL license text Signed-off-by: Chunfeng Yun --- v2: change subject line to fix checkpatch warning: "A patch subject line should describe the change not the tool that found it" --- drivers/phy/mediatek/Makefile | 1 + drivers/phy/mediatek/phy-mtk-tphy.c | 10 +- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile index 763a92e..1bdab14 100644 --- a/drivers/phy/mediatek/Makefile +++ b/drivers/phy/mediatek/Makefile @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: GPL-2.0 # # Makefile for the phy drivers. # diff --git a/drivers/phy/mediatek/phy-mtk-tphy.c b/drivers/phy/mediatek/phy-mtk-tphy.c index 38c281b..b962339 100644 --- a/drivers/phy/mediatek/phy-mtk-tphy.c +++ b/drivers/phy/mediatek/phy-mtk-tphy.c @@ -1,16 +1,8 @@ +// SPDX-License-Identifier: GPL-2.0 /* * Copyright (c) 2015 MediaTek Inc. * Author: Chunfeng Yun * - * This software is licensed under the terms of the GNU General Public - * License version 2, as published by the Free Software Foundation, and - * may be copied, distributed, and modified under those terms. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * */ #include -- 1.9.1
Applied "regulator: add dummy function of_find_regulator_by_node" to the regulator tree
The patch regulator: add dummy function of_find_regulator_by_node has been applied to the regulator tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 08813e0ec1cb48e53c86a24d88d26b26878e7b6e Mon Sep 17 00:00:00 2001 From: Changbin Du Date: Wed, 2 May 2018 21:44:57 +0800 Subject: [PATCH] regulator: add dummy function of_find_regulator_by_node If device tree is not enabled, of_find_regulator_by_node() should have a dummy function since the function call is still there. This is to fix build error after CONFIG_NO_AUTO_INLINE is introduced. If this option is enabled, GCC will not auto-inline functions that are not explicitly marked as inline. In this case (no CONFIG_OF), the copmiler will report error in function regulator_dev_lookup(). W/O NO_AUTO_INLINE, function of_get_regulator() is auto-inlined and then the call to of_find_regulator_by_node() is optimized out since of_get_regulator() always return NULL. W/ NO_AUTO_INLINE, the return value of of_get_regulator() is a variable so the call to of_find_regulator_by_node() cannot be optimized out. So we need a stub of_find_regulator_by_node(). static struct regulator_dev *regulator_dev_lookup(struct device *dev, const char *supply) { struct regulator_dev *r = NULL; struct device_node *node; struct regulator_map *map; const char *devname = NULL; regulator_supply_alias(&dev, &supply); /* first do a dt based lookup */ if (dev && dev->of_node) { node = of_get_regulator(dev, supply); if (node) { r = of_find_regulator_by_node(node); if (r) return r; ... Signed-off-by: Changbin Du Signed-off-by: Mark Brown --- drivers/regulator/internal.h | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/regulator/internal.h b/drivers/regulator/internal.h index abfd56e8c78a..24fde1e08f3a 100644 --- a/drivers/regulator/internal.h +++ b/drivers/regulator/internal.h @@ -56,14 +56,19 @@ static inline struct regulator_dev *dev_to_rdev(struct device *dev) return container_of(dev, struct regulator_dev, dev); } -struct regulator_dev *of_find_regulator_by_node(struct device_node *np); - #ifdef CONFIG_OF +struct regulator_dev *of_find_regulator_by_node(struct device_node *np); struct regulator_init_data *regulator_of_get_init_data(struct device *dev, const struct regulator_desc *desc, struct regulator_config *config, struct device_node **node); #else +static inline struct regulator_dev * +of_find_regulator_by_node(struct device_node *np) +{ + return NULL; +} + static inline struct regulator_init_data * regulator_of_get_init_data(struct device *dev, const struct regulator_desc *desc, -- 2.17.0
Applied "regulator: pfuze100: Make the node name generic" to the regulator tree
The patch regulator: pfuze100: Make the node name generic has been applied to the regulator tree at https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regulator.git All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark >From 60891ceb8075e02cbf841ac1e7f7437d711ffc5f Mon Sep 17 00:00:00 2001 From: Fabio Estevam Date: Fri, 4 May 2018 22:17:13 -0300 Subject: [PATCH] regulator: pfuze100: Make the node name generic According to Devicetree Specification v0.2 document: "The name of a node should be somewhat generic, reflecting the function of the device and not its precise programming model." Do as suggested in the binding example. Signed-off-by: Fabio Estevam Signed-off-by: Mark Brown --- Documentation/devicetree/bindings/regulator/pfuze100.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/devicetree/bindings/regulator/pfuze100.txt b/Documentation/devicetree/bindings/regulator/pfuze100.txt index c6dd3f5e485b..f0ada3b14d70 100644 --- a/Documentation/devicetree/bindings/regulator/pfuze100.txt +++ b/Documentation/devicetree/bindings/regulator/pfuze100.txt @@ -21,7 +21,7 @@ Each regulator is defined using the standard binding for regulators. Example 1: PFUZE100 - pmic: pfuze100@8 { + pfuze100: pmic@8 { compatible = "fsl,pfuze100"; reg = <0x08>; @@ -122,7 +122,7 @@ Example 1: PFUZE100 Example 2: PFUZE200 - pmic: pfuze200@8 { + pfuze200: pmic@8 { compatible = "fsl,pfuze200"; reg = <0x08>; @@ -216,7 +216,7 @@ Example 2: PFUZE200 Example 3: PFUZE3000 - pmic: pfuze3000@8 { + pfuze3000: pmic@8 { compatible = "fsl,pfuze3000"; reg = <0x08>; -- 2.17.0
[PATCH] i2c: core-smbus: fix a potential uninitialization bug
In i2c_smbus_xfer_emulated(), there are two buffers: msgbuf0 and msgbuf1, which are used to save a series of messages, as mentioned in the comment. According to the value of the variable "size", msgbuf0 is initialized to various values. In contrast, msgbuf1 is left uninitialized until the function i2c_transfer() is invoked. However, mgsbuf1 is not always initialized on all possible execution paths (implementation) of i2c_transfer(). Thus, it is possible that mgsbuf1 may still be uninitialized even after the invocation of the function i2c_transfer(), especially when the return value of ic2_transfer() is not checked properly. In the following execution, the uninitialized msgbuf1 will be used, such as for security checks. Since uninitialized values can be random and arbitrary, this will cause undefined behaviors or even check bypass. For example, it is expected that if the value of "size" is I2C_SMBUS_BLOCK_PROC_CALL, the value of data->block[0] should not be larger than I2C_SMBUS_BLOCK_MAX. But, at the end of i2c_smbus_xfer_emulated(), the value read from msgbuf1 is assigned to data->block[0], which can potentially lead to invalid block write size, as demonstrated in the error message. This patch checks the return value of i2c_transfer() and also initializes the first byte of msgbuf1 with 0 to avoid undefined behaviors or security issues. Signed-off-by: Wenwen Wang --- drivers/i2c/i2c-core-smbus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c index b5aec33..e8470d5 100644 --- a/drivers/i2c/i2c-core-smbus.c +++ b/drivers/i2c/i2c-core-smbus.c @@ -344,6 +344,7 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr, }; msgbuf0[0] = command; + msgbug1[0] = 0; switch (size) { case I2C_SMBUS_QUICK: msg[0].len = 0; @@ -466,6 +467,8 @@ static s32 i2c_smbus_xfer_emulated(struct i2c_adapter *adapter, u16 addr, status = i2c_transfer(adapter, msg, num); if (status < 0) return status; + if (status != num) + return -EIO; /* Check PEC if last message is a read */ if (i && (msg[num-1].flags & I2C_M_RD)) { -- 2.7.4
Re: [PATCH] MAINTAINERS: Update Kbuild entry with a few paths
2018-03-23 5:58 GMT+09:00 Rasmus Villemoes : > I managed to send some modpost patches to old addresses of both > Masahiro and Michal, and omitted linux-kbuild from cc, because my > tried and trusted scripts/get_maintainer wrapper failed me. Add the > modpost directory to the MAINTAINERS entry, and while at it make the > Makefile glob match scripts/Makefile itself, and add one matching the > Kbuild.include file as well. > > Signed-off-by: Rasmus Villemoes > --- > Hi Kbuild maintainers. Hope you don't mind me extending this entry a bit. Applied to linux-kbuild/fixes. Thanks. -- Best Regards Masahiro Yamada
Re: [PATCH v2 net-next 1/4] umh: introduce fork_usermode_blob() helper
On Fri, May 04, 2018 at 07:56:43PM +, Luis R. Rodriguez wrote: > What a mighty short list of reviewers. Adding some more. My review below. > I'd appreciate a Cc on future versions of these patches. sure. > On Wed, May 02, 2018 at 09:36:01PM -0700, 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" > > Actually its a *shared* vfsmount tmpfs for all umh blobs. yep > > - 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 > > But since its using UMH_WAIT_EXEC, all we can guarantee currently is the > inception point was intended, well though out, and will run, but the return > value in no way reflects the success or not of the execution. More below. yep > > 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. > > > > Such approach enables kernel to delegate functionality traditionally done > > by the kernel modules into the user space processes (either root or !root) > > and > > reduces security attack surface of the new code. The buggy umh code would > > crash > > the user process, but not the kernel. Another advantage is that umh code > > of the kernel module can be debugged and tested out of user space > > (e.g. opening the possibility to run clang sanitizers, fuzzers or > > user space test suites on the umh code). > > In case of the bpfilter project such architecture allows complex control > > plane > > to be done in the user space while bpf based data plane stays in the kernel. > > > > Since umh can crash, can be oom-ed by the kernel, killed by the admin, > > the kernel module that uses them (like bpfilter) needs to manage life > > time of umh on its own via two unix pipes and the pid of umh. > > > > The exit code of such kernel module should kill the umh it started, > > so that rmmod of the kernel module will cleanup the corresponding umh. > > Just like if the kernel module does kmalloc() it should kfree() it in the > > exit code. > > > > Signed-off-by: Alexei Starovoitov > > --- > > fs/exec.c | 38 --- > > include/linux/binfmts.h | 1 + > > include/linux/umh.h | 12 > > kernel/umh.c| 176 > > +++- > > 4 files changed, 215 insertions(+), 12 deletions(-) > > > > diff --git a/fs/exec.c b/fs/exec.c > > index 183059c427b9..30a36c2a39bf 100644 > > --- a/fs/exec.c > > +++ b/fs/exec.c > > @@ -1706,14 +1706,13 @@ static int exec_binprm(struct linux_binprm *bprm) > > /* > > * sys_execve() executes a new program. > > */ > > -static int do_execveat_common(int fd, struct filename *filename, > > - struct user_arg_ptr argv, > > - struct user_arg_ptr envp, > > - int flags) > > +static int __do_execve_file(int fd, struct filename *filename, > > + struct user_arg_ptr argv, > > + struct user_arg_ptr envp, > > + int flags, struct file *file) > > { > > char *pathbuf = NULL; > > struct linux_binprm *bprm; > > - struct file *file; > > struct files_struct *displaced; > > int retval; > > Keeping in mind a fuzzer... > > Note, right below this, and not shown here in the hunk, is: > > if (IS_ERR(filename)) > > return PTR_ERR(filename) > > > > @@ -1752,7 +1751,8 @@ static int do_execveat_common(int fd, struct filename > > *filename, > > check_unsafe_exec(bprm); > > current->in_execve = 1; > > > > - file = do_open_execat(fd, filename, flags); > > + i
Re: [PATCH 01/30] gcc-plugins: fix build condition of SANCOV plugin
2018-05-05 1:21 GMT+09:00 Kees Cook : > On Fri, May 4, 2018 at 7:21 AM, Masahiro Yamada > wrote: >> Hi Kees, >> >> >> 2018-04-13 14:06 GMT+09:00 Masahiro Yamada : >>> Since commit d677a4d60193 ("Makefile: support flag >>> -fsanitizer-coverage=trace-cmp"), you miss to build the SANCOV >>> plugin under some circumstances. >>> >>> CONFIG_KCOV=y >>> CONFIG_KCOV_ENABLE_COMPARISONS=y >>> Your compiler does not support -fsanitize-coverage=trace-pc >>> Your compiler does not support -fsanitize-coverage=trace-cmp >>> >>> Under this condition, $(CFLAGS_KCOV) is not empty but contains a >>> space, so the following ifeq-conditional is false. >>> >>> ifeq ($(CFLAGS_KCOV),) >>> >>> Then, scripts/Makefile.gcc-plugins misses to add sancov_plugin.so to >>> gcc-plugin-y while the SANCOV plugin is necessary as an alternative >>> means. >>> >>> Fixes: d677a4d60193 ("Makefile: support flag >>> -fsanitizer-coverage=trace-cmp") >>> Signed-off-by: Masahiro Yamada >>> --- >> >> >> I am planning to queue this up to the fixes branch >> since this is a bug fix. >> >> Do you have any comment on this? > > Looks fine to me; thanks! > > Acked-by: Kees Cook > > -Kees > >> Applied to linux-kbuild/fixes. -- Best Regards Masahiro Yamada
Re: [PATCH] i2c: core-smbus: fix a potential uninitialization bug
On Fri, May 4, 2018 at 10:38 AM, Peter Rosin wrote: > On 2018-05-04 16:59, Wenwen Wang wrote: >> On Fri, May 4, 2018 at 2:27 AM, Peter Rosin wrote: >>> On 2018-05-04 09:17, Wenwen Wang wrote: On Fri, May 4, 2018 at 1:49 AM, Peter Rosin wrote: > On 2018-05-04 07:28, Wenwen Wang wrote: >> On Fri, May 4, 2018 at 12:04 AM, Peter Rosin wrote: >>> On 2018-05-04 06:08, Wenwen Wang wrote: On Thu, May 3, 2018 at 3:34 PM, Peter Rosin wrote: > On 2018-05-03 00:36, Wenwen Wang wrote: >> In i2c_smbus_xfer_emulated(), there are two buffers: msgbuf0 and >> msgbuf1, >> which are used to save a series of messages, as mentioned in the >> comment. >> According to the value of the variable "size", msgbuf0 is >> initialized to >> various values. In contrast, msgbuf1 is left uninitialized until the >> function i2c_transfer() is invoked. However, mgsbuf1 is not always >> initialized on all possible execution paths (implementation) of >> i2c_transfer(). Thus, it is possible that mgsbuf1 may still not be > > double negation here > >> uninitialized even after the invocation of the function >> i2c_transfer(). In >> the following execution, the uninitialized msgbuf1 will be used, >> such as >> for security checks. Since uninitialized values can be random and >> arbitrary, this will cause undefined behaviors or even check bypass. >> For >> example, it is expected that if the value of "size" is >> I2C_SMBUS_BLOCK_PROC_CALL, the value of data->block[0] should not be >> larger >> than I2C_SMBUS_BLOCK_MAX. But, at the end of >> i2c_smbus_xfer_emulated(), the >> value read from msgbuf1 is assigned to data->block[0], which can >> potentially lead to invalid block write size, as demonstrated in the >> error >> message. >> >> This patch simply initializes the buffer msgbuf1 with 0 to avoid >> undefined >> behaviors or security issues. >> >> Signed-off-by: Wenwen Wang >> --- >> drivers/i2c/i2c-core-smbus.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/i2c/i2c-core-smbus.c >> b/drivers/i2c/i2c-core-smbus.c >> index b5aec33..0fcca75 100644 >> --- a/drivers/i2c/i2c-core-smbus.c >> +++ b/drivers/i2c/i2c-core-smbus.c >> @@ -324,7 +324,7 @@ static s32 i2c_smbus_xfer_emulated(struct >> i2c_adapter *adapter, u16 addr, >>* somewhat simpler. >>*/ >> unsigned char msgbuf0[I2C_SMBUS_BLOCK_MAX+3]; >> - unsigned char msgbuf1[I2C_SMBUS_BLOCK_MAX+2]; >> + unsigned char msgbuf1[I2C_SMBUS_BLOCK_MAX+2] = {0}; > > I think this will result in the whole of msgbuf1 being filled with > zeroes. > It might be cheaper to do this with code proper rather than with an > initializer? Thanks for your comment, Peter! How about using a memset() only when i2c_smbus_xfer_emulated() emulates reading commands, since msgbuf1 is used only in that case? >>> >>> I was thinking that an assignment of >>> >>> msgbuf1[0] = 0; >>> >>> would be enough in the I2C_SMBUS_BLOCK_DATA and >>> I2C_SMBUS_BLOCK_PROC_CALL >>> cases before the i2c_transfer call. However, this will only kick in if >>> the call to kzalloc fails (and it most likely will not) in the call to >>> the >>> i2c_smbus_try_get_dmabuf helper. So, this thing that you are trying to >>> fix >>> seems like a non-issue to me. >>> >>> However, while looking I think the bigger problem with that function is >>> that >>> it considers all non-negative return values from i2c_transfer as >>> good. >>> IMHO, it should barf on any return values <> num. Or at the very least >>> describe why a partial result is considered OK... >>> >>> Cheers, >>> Peter >>> > > Cheers, > Peter > >> int num = read_write == I2C_SMBUS_READ ? 2 : 1; >> int i; >> u8 partial_pec = 0; >> > >>> >> >> Yes, it is a big issue if the return value from i2c_transfer() is not >> equal to num. I can add a check like this: >> >> if (status != num) >> return -EINVAL; >> > > Right, but make sure to add it *after* the existing "if (status < 0)" > check as we want to preserve any existing error. Also, -EIO is perhaps > more appropriate than -EINVAL which seems wrong for what is probably > a runtime incident. > Sure, I will place it after the existing check and replace -EINVAL with -EIO. >> Also, I wonder why ms
Re: [RFC PATCH] driver core: make deferring probe forever optional
On Wed, May 02, 2018 at 07:49:57PM +0100, Robin Murphy wrote: > I guess there's also the possibility that a single driver may want multiple > behaviours, if e.g. if SoC variants A and B have some identical peripherals > but slightly different pinctrl/IOMMU/etc. hardware such that A has workable > default behaviour and can be treated as optional, whereas B absolutely must > be controlled by the kernel for the consumers to function properly, and they > *should* defer forever otherwise. I think that would pretty much demand some > sort of explicitly-curated white/blacklist setup at the subsystem or driver > level. Different board variants, and possibly even different bootloaders might also be an issue here - a vendor bootloader might do pinmuxing that an upstream bootloader doesn't for example. In some cases the pinmuxing even depends on the boot method with things only getting configured if the bootloader wanted to use them. signature.asc Description: PGP signature
[PATCH 24/24] selftests: net: return Kselftest Skip code for skipped tests
When net test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. Kselftest framework SKIP code is 4 and the framework prints appropriate messages to indicate that the test is skipped. Change psock_tpacket to use ksft_exit_skip() when a non-root user runs the test and add an explicit check for root and a clear message, instead of failing the test when /sys/power/state file open fails. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/net/fib_tests.sh| 8 +--- tools/testing/selftests/net/netdevice.sh| 16 +-- tools/testing/selftests/net/pmtu.sh | 5 - tools/testing/selftests/net/psock_tpacket.c | 4 +++- tools/testing/selftests/net/rtnetlink.sh| 31 - 5 files changed, 39 insertions(+), 25 deletions(-) diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh index 9164e60d4b66..5baac82b9287 100755 --- a/tools/testing/selftests/net/fib_tests.sh +++ b/tools/testing/selftests/net/fib_tests.sh @@ -5,6 +5,8 @@ # different events. ret=0 +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 VERBOSE=${VERBOSE:=0} PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no} @@ -579,18 +581,18 @@ fib_test() if [ "$(id -u)" -ne 0 ];then echo "SKIP: Need root privileges" - exit 0 + exit $ksft_skip; fi if [ ! -x "$(command -v ip)" ]; then echo "SKIP: Could not run test without ip tool" - exit 0 + exit $ksft_skip fi ip route help 2>&1 | grep -q fibmatch if [ $? -ne 0 ]; then echo "SKIP: iproute2 too old, missing fibmatch" - exit 0 + exit $ksft_skip fi # start clean diff --git a/tools/testing/selftests/net/netdevice.sh b/tools/testing/selftests/net/netdevice.sh index 903679e0ff31..e3afcb424710 100755 --- a/tools/testing/selftests/net/netdevice.sh +++ b/tools/testing/selftests/net/netdevice.sh @@ -8,6 +8,9 @@ # if not they probably have failed earlier in the boot process and their logged error will be catched by another test # +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # this function will try to up the interface # if already up, nothing done # arg1: network interface name @@ -18,7 +21,7 @@ kci_net_start() ip link show "$netdev" |grep -q UP if [ $? -eq 0 ];then echo "SKIP: $netdev: interface already up" - return 0 + return $ksft_skip fi ip link set "$netdev" up @@ -61,12 +64,12 @@ kci_net_setup() ip address show "$netdev" |grep '^[[:space:]]*inet' if [ $? -eq 0 ];then echo "SKIP: $netdev: already have an IP" - return 0 + return $ksft_skip fi # TODO what ipaddr to set ? DHCP ? echo "SKIP: $netdev: set IP address" - return 0 + return $ksft_skip } # test an ethtool command @@ -84,6 +87,7 @@ kci_netdev_ethtool_test() if [ $ret -ne 0 ];then if [ $ret -eq "$1" ];then echo "SKIP: $netdev: ethtool $2 not supported" + return $ksft_skip else echo "FAIL: $netdev: ethtool $2" return 1 @@ -104,7 +108,7 @@ kci_netdev_ethtool() ethtool --version 2>/dev/null >/dev/null if [ $? -ne 0 ];then echo "SKIP: ethtool not present" - return 1 + return $ksft_skip fi TMP_ETHTOOL_FEATURES="$(mktemp)" @@ -176,13 +180,13 @@ kci_test_netdev() #check for needed privileges if [ "$(id -u)" -ne 0 ];then echo "SKIP: Need root privileges" - exit 0 + exit $ksft_skip fi ip link show 2>/dev/null >/dev/null if [ $? -ne 0 ];then echo "SKIP: Could not run test without the ip tool" - exit 0 + exit $ksft_skip fi TMP_LIST_NETDEV="$(mktemp)" diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh index 1e428781a625..7514f93e1624 100755 --- a/tools/testing/selftests/net/pmtu.sh +++ b/tools/testing/selftests/net/pmtu.sh @@ -43,6 +43,9 @@ # that MTU is properly calculated instead when MTU is not configured from # userspace +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + tests=" pmtu_vti6_exception vti6: PMTU exceptions pmtu_vti4_exception vti4: PMTU exceptions @@ -162,7 +165,7 @@ setup_xfrm6() { } setup() { - [ "$(id -u)" -ne 0 ] && echo " need to run as root" && return 1 + [ "$(id -u)" -ne 0 ] && echo " need to run as root" && return $ksft_skip cleanup_done=0 for arg do diff --git a/tool
[PATCH 23/24] selftests: mqueue: return Kselftest Skip code for skipped tests
When mqueue test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when the test is skipped. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/mqueue/mq_open_tests.c | 8 tools/testing/selftests/mqueue/mq_perf_tests.c | 8 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/mqueue/mq_open_tests.c b/tools/testing/selftests/mqueue/mq_open_tests.c index 677140aa25fd..9403ac01ba11 100644 --- a/tools/testing/selftests/mqueue/mq_open_tests.c +++ b/tools/testing/selftests/mqueue/mq_open_tests.c @@ -33,6 +33,8 @@ #include #include +#include "../kselftest.h" + static char *usage = "Usage:\n" " %s path\n" @@ -262,12 +264,10 @@ int main(int argc, char *argv[]) } } - if (getuid() != 0) { - fprintf(stderr, "Not running as root, but almost all tests " + if (getuid() != 0) + ksft_exit_skip("Not running as root, but almost all tests " "require root in order to modify\nsystem settings. " "Exiting.\n"); - exit(1); - } /* Find out what files there are for us to make tweaks in */ def_msgs = fopen(DEF_MSGS, "r+"); diff --git a/tools/testing/selftests/mqueue/mq_perf_tests.c b/tools/testing/selftests/mqueue/mq_perf_tests.c index 8188f72de93c..b019e0b8221c 100644 --- a/tools/testing/selftests/mqueue/mq_perf_tests.c +++ b/tools/testing/selftests/mqueue/mq_perf_tests.c @@ -39,6 +39,8 @@ #include #include +#include "../kselftest.h" + static char *usage = "Usage:\n" " %s [-c #[,#..] -f] path\n" @@ -626,12 +628,10 @@ int main(int argc, char *argv[]) cpus_to_pin[0] = cpus_online - 1; } - if (getuid() != 0) { - fprintf(stderr, "Not running as root, but almost all tests " + if (getuid() != 0) + ksft_exit_skip("Not running as root, but almost all tests " "require root in order to modify\nsystem settings. " "Exiting.\n"); - exit(1); - } max_msgs = fopen(MAX_MSGS, "r+"); max_msgsize = fopen(MAX_MSGSIZE, "r+"); -- 2.14.1
[PATCH 21/24] selftests: memfd: return Kselftest Skip code for skipped tests
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 fi while read name size unit; do if [ "$name" = "HugePages_Free:" ]; then -- 2.14.1
[PATCH 22/24] selftests: memory-hotplug: return Kselftest Skip code for skipped tests
When memory-hotplug test is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value hich 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. 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/memory-hotplug/mem-on-off-test.sh | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh index ff4991704d07..b37585e6aa38 100755 --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh @@ -3,30 +3,33 @@ SYSFS= +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + prerequisite() { msg="skip all tests:" if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'` if [ ! -d "$SYSFS" ]; then echo $msg sysfs is not mounted >&2 - exit 0 + exit $ksft_skip fi if ! ls $SYSFS/devices/system/memory/memory* > /dev/null 2>&1; then echo $msg memory hotplug is not supported >&2 - exit 0 + exit $ksft_skip fi if ! grep -q 1 $SYSFS/devices/system/memory/memory*/removable; then echo $msg no hot-pluggable memory >&2 - exit 0 + exit $ksft_skip fi } -- 2.14.1
[PATCH 20/24] selftests: membarrier: return Kselftest Skip code for skipped tests
When membarrier test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when the test is skipped. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/membarrier/membarrier_test.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/membarrier/membarrier_test.c b/tools/testing/selftests/membarrier/membarrier_test.c index 22bffd55a523..6793f8ecc8e7 100644 --- a/tools/testing/selftests/membarrier/membarrier_test.c +++ b/tools/testing/selftests/membarrier/membarrier_test.c @@ -293,10 +293,9 @@ static int test_membarrier_query(void) } ksft_exit_fail_msg("sys_membarrier() failed\n"); } - if (!(ret & MEMBARRIER_CMD_GLOBAL)) { - ksft_test_result_fail("sys_membarrier() CMD_GLOBAL query failed\n"); - ksft_exit_fail_msg("sys_membarrier is not supported.\n"); - } + if (!(ret & MEMBARRIER_CMD_GLOBAL)) + ksft_exit_skip( + "sys_membarrier unsupported: CMD_GLOBAL not found.\n"); ksft_test_result_pass("sys_membarrier available\n"); return 0; -- 2.14.1
[PATCH 19/24] selftests: media_tests: return Kselftest Skip code for skipped tests
When media_tests test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when the test is skipped. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/media_tests/Makefile| 3 ++- tools/testing/selftests/media_tests/media_device_open.c | 8 tools/testing/selftests/media_tests/media_device_test.c | 8 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/tools/testing/selftests/media_tests/Makefile b/tools/testing/selftests/media_tests/Makefile index c82cec2497de..60826d7d37d4 100644 --- a/tools/testing/selftests/media_tests/Makefile +++ b/tools/testing/selftests/media_tests/Makefile @@ -1,5 +1,6 @@ # SPDX-License-Identifier: GPL-2.0 +# +CFLAGS += -I../ -I../../../../usr/include/ TEST_GEN_PROGS := media_device_test media_device_open video_device_test -all: $(TEST_GEN_PROGS) include ../lib.mk diff --git a/tools/testing/selftests/media_tests/media_device_open.c b/tools/testing/selftests/media_tests/media_device_open.c index a5ce5434bafd..93183a37b133 100644 --- a/tools/testing/selftests/media_tests/media_device_open.c +++ b/tools/testing/selftests/media_tests/media_device_open.c @@ -34,6 +34,8 @@ #include #include +#include "../kselftest.h" + int main(int argc, char **argv) { int opt; @@ -61,10 +63,8 @@ int main(int argc, char **argv) } } - if (getuid() != 0) { - printf("Please run the test as root - Exiting.\n"); - exit(-1); - } + if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n"); /* Open Media device and keep it open */ fd = open(media_device, O_RDWR); diff --git a/tools/testing/selftests/media_tests/media_device_test.c b/tools/testing/selftests/media_tests/media_device_test.c index 421a367e4bb3..a4c6ad18cb5c 100644 --- a/tools/testing/selftests/media_tests/media_device_test.c +++ b/tools/testing/selftests/media_tests/media_device_test.c @@ -39,6 +39,8 @@ #include #include +#include "../kselftest.h" + int main(int argc, char **argv) { int opt; @@ -66,10 +68,8 @@ int main(int argc, char **argv) } } - if (getuid() != 0) { - printf("Please run the test as root - Exiting.\n"); - exit(-1); - } + if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n"); /* Generate random number of interations */ srand((unsigned int) time(NULL)); -- 2.14.1
[PATCH 17/24] selftests: locking: add Makefile for locking test
Add Makefile for locking test. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/locking/Makefile | 10 ++ 1 file changed, 10 insertions(+) create mode 100644 tools/testing/selftests/locking/Makefile diff --git a/tools/testing/selftests/locking/Makefile b/tools/testing/selftests/locking/Makefile new file mode 100644 index ..e168a2e34bc2 --- /dev/null +++ b/tools/testing/selftests/locking/Makefile @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Makefile for locking/ww_mutx selftests + +# No binaries, but make sure arg-less "make" doesn't trigger "run_tests" +all: + +TEST_PROGS := ww_mutex.sh + +include ../lib.mk -- 2.14.1
[PATCH 18/24] selftests: locking: return Kselftest Skip code for skipped tests
When locking test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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 search for ww_mutex module and return skip code if it isn't found to differentiate between the failure to load the module condition and module not found condition. 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/locking/ww_mutex.sh | 8 1 file changed, 8 insertions(+) mode change 100644 => 100755 tools/testing/selftests/locking/ww_mutex.sh diff --git a/tools/testing/selftests/locking/ww_mutex.sh b/tools/testing/selftests/locking/ww_mutex.sh old mode 100644 new mode 100755 index 2c3d6b1878c2..91e4ac7566af --- a/tools/testing/selftests/locking/ww_mutex.sh +++ b/tools/testing/selftests/locking/ww_mutex.sh @@ -1,6 +1,14 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # Runs API tests for struct ww_mutex (Wait/Wound mutexes) +if ! /sbin/modprobe -q -n test-ww_mutex; then + echo "ww_mutex: module test-ww_mutex is not found [SKIP]" + exit $ksft_skip +fi if /sbin/modprobe -q test-ww_mutex; then /sbin/modprobe -q -r test-ww_mutex -- 2.14.1
[PATCH 16/24] selftests: lib: return Kselftest Skip code for skipped tests
When lib test(s) is skipped because of unmet dependencies and/or unsupported configuration, it returns non-zero value hich 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. 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/lib/bitmap.sh| 8 ++-- tools/testing/selftests/lib/prime_numbers.sh | 7 +-- tools/testing/selftests/lib/printf.sh| 8 ++-- 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/lib/bitmap.sh b/tools/testing/selftests/lib/bitmap.sh index 4dee4d2a8bbe..5a90006d1aea 100755 --- a/tools/testing/selftests/lib/bitmap.sh +++ b/tools/testing/selftests/lib/bitmap.sh @@ -1,9 +1,13 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + # Runs bitmap infrastructure tests using test_bitmap kernel module if ! /sbin/modprobe -q -n test_bitmap; then - echo "bitmap: [SKIP]" - exit 77 + echo "bitmap: module test_bitmap is not found [SKIP]" + exit $ksft_skip fi if /sbin/modprobe -q test_bitmap; then diff --git a/tools/testing/selftests/lib/prime_numbers.sh b/tools/testing/selftests/lib/prime_numbers.sh index b363994e5e11..76602d4b050f 100755 --- a/tools/testing/selftests/lib/prime_numbers.sh +++ b/tools/testing/selftests/lib/prime_numbers.sh @@ -2,9 +2,12 @@ # SPDX-License-Identifier: GPL-2.0 # Checks fast/slow prime_number generation for inconsistencies +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + if ! /sbin/modprobe -q -r prime_numbers; then - echo "prime_numbers: [SKIP]" - exit 77 + echo "prime_numbers: module prime_numbers is not found [SKIP]" + exit $ksft_skip fi if /sbin/modprobe -q prime_numbers selftest=65536; then diff --git a/tools/testing/selftests/lib/printf.sh b/tools/testing/selftests/lib/printf.sh index 0c37377fd7d4..45a23e2d64ad 100755 --- a/tools/testing/selftests/lib/printf.sh +++ b/tools/testing/selftests/lib/printf.sh @@ -1,9 +1,13 @@ #!/bin/sh # SPDX-License-Identifier: GPL-2.0 # Runs printf infrastructure using test_printf kernel module + +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + if ! /sbin/modprobe -q -n test_printf; then - echo "printf: [SKIP]" - exit 77 + echo "printf: module test_printf is not found [SKIP]" + exit $ksft_skip fi if /sbin/modprobe -q test_printf; then -- 2.14.1
[PATCH 11/24] selftests: intel_pstate: return Kselftest Skip code for skipped tests
When intel_pstate test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. 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/intel_pstate/run.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/intel_pstate/run.sh b/tools/testing/selftests/intel_pstate/run.sh index c670359becc6..6ded61670f6d 100755 --- a/tools/testing/selftests/intel_pstate/run.sh +++ b/tools/testing/selftests/intel_pstate/run.sh @@ -30,9 +30,12 @@ EVALUATE_ONLY=0 +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + if ! uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ | grep -q x86; then echo "$0 # Skipped: Test can only run on x86 architectures." - exit 0 + exit $ksft_skip fi max_cpus=$(($(nproc)-1)) -- 2.14.1
[PATCH 12/24] selftests: ipc: return Kselftest Skip code for skipped tests
When ipc test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when the test is skipped. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/ipc/msgque.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/ipc/msgque.c b/tools/testing/selftests/ipc/msgque.c index ee9382bdfadc..dac927e82336 100644 --- a/tools/testing/selftests/ipc/msgque.c +++ b/tools/testing/selftests/ipc/msgque.c @@ -196,10 +196,9 @@ int main(int argc, char **argv) int msg, pid, err; struct msgque_data msgque; - if (getuid() != 0) { - printf("Please run the test as root - Exiting.\n"); - return ksft_exit_fail(); - } + if (getuid() != 0) + return ksft_exit_skip( + "Please run the test as root - Exiting.\n"); msgque.key = ftok(argv[0], 822155650); if (msgque.key == -1) { -- 2.14.1
[PATCH 15/24] selftests: lib: add prime_numbers.sh test to Makefile
prime_numbers.sh is not included in TEST_PROGS. Add it. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/lib/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/lib/Makefile b/tools/testing/selftests/lib/Makefile index 08360060ab14..70d5711e3ac8 100644 --- a/tools/testing/selftests/lib/Makefile +++ b/tools/testing/selftests/lib/Makefile @@ -3,6 +3,6 @@ # No binaries, but make sure arg-less "make" doesn't trigger "run_tests" all: -TEST_PROGS := printf.sh bitmap.sh +TEST_PROGS := printf.sh bitmap.sh prime_numbers.sh include ../lib.mk -- 2.14.1
[PATCH 13/24] selftests: kmod: return Kselftest Skip code for skipped tests
When kmod test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive result even when the test could not be run. It returns fail in some cases when test is skipped. Either way, it is incorrect and incosnistent reporting. Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run. 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/kmod/kmod.sh | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/kmod/kmod.sh b/tools/testing/selftests/kmod/kmod.sh index 7956ea3be667..0a76314b4414 100755 --- a/tools/testing/selftests/kmod/kmod.sh +++ b/tools/testing/selftests/kmod/kmod.sh @@ -62,13 +62,16 @@ ALL_TESTS="$ALL_TESTS 0007:5:1" ALL_TESTS="$ALL_TESTS 0008:150:1" ALL_TESTS="$ALL_TESTS 0009:150:1" +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + test_modprobe() { if [ ! -d $DIR ]; then echo "$0: $DIR not present" >&2 echo "You must have the following enabled in your kernel:" >&2 cat $TEST_DIR/config >&2 - exit 1 + exit $ksft_skip fi } @@ -105,12 +108,12 @@ test_reqs() { if ! which modprobe 2> /dev/null > /dev/null; then echo "$0: You need modprobe installed" >&2 - exit 1 + exit $ksft_skip fi if ! which kmod 2> /dev/null > /dev/null; then echo "$0: You need kmod installed" >&2 - exit 1 + exit $ksft_skip fi # kmod 19 has a bad bug where it returns 0 when modprobe @@ -124,13 +127,13 @@ test_reqs() echo "$0: You need at least kmod 20" >&2 echo "kmod <= 19 is buggy, for details see:" >&2 echo "http://git.kernel.org/cgit/utils/kernel/kmod/kmod.git/commit/libkmod/libkmod-module.c?id=fd44a98ae2eb5eb32161088954ab21e58e19dfc4"; >&2 - exit 1 + exit $ksft_skip fi uid=$(id -u) if [ $uid -ne 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi } -- 2.14.1
[PATCH 14/24] selftests: kvm: return Kselftest Skip code for skipped tests
When kvm test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when the test is skipped. In addition, refine test_assert() message to include strerror() string and add explicit check for root user to clearly identofy non-root user skip case. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/kvm/lib/assert.c | 10 -- tools/testing/selftests/kvm/vmx_tsc_adjust_test.c | 4 +++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/testing/selftests/kvm/lib/assert.c b/tools/testing/selftests/kvm/lib/assert.c index c9f5b7d4ce38..4705729d847e 100644 --- a/tools/testing/selftests/kvm/lib/assert.c +++ b/tools/testing/selftests/kvm/lib/assert.c @@ -13,6 +13,8 @@ #include #include +#include "../../kselftest.h" + /* Dumps the current stack trace to stderr. */ static void __attribute__((noinline)) test_dump_stack(void); static void test_dump_stack(void) @@ -65,13 +67,17 @@ test_assert(bool exp, const char *exp_str, { va_list ap; + if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n"); + if (!(exp)) { va_start(ap, fmt); fprintf(stderr, " Test Assertion Failure \n" " %s:%u: %s\n" - " pid=%d tid=%d\n", - file, line, exp_str, getpid(), gettid()); + " pid=%d tid=%d - %s\n", + file, line, exp_str, getpid(), gettid(), + strerror(errno)); test_dump_stack(); if (fmt) { fputs(" ", stderr); diff --git a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c index 8f7f62093add..62fb73699eb6 100644 --- a/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c +++ b/tools/testing/selftests/kvm/vmx_tsc_adjust_test.c @@ -28,6 +28,8 @@ #include #include +#include "../kselftest.h" + #ifndef MSR_IA32_TSC_ADJUST #define MSR_IA32_TSC_ADJUST 0x3b #endif @@ -190,7 +192,7 @@ int main(int argc, char *argv[]) if (!(entry->ecx & CPUID_VMX)) { printf("nested VMX not enabled, skipping test"); - return 0; + return KSFT_SKIP; } vm = vm_create_default_vmx(VCPU_ID, (void *) l1_guest_code); -- 2.14.1
[PATCH 10/24] selftests: gpio: return Kselftest Skip code for skipped tests
When gpio test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. 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/gpio/gpio-mockup.sh | 12 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/gpio/gpio-mockup.sh b/tools/testing/selftests/gpio/gpio-mockup.sh index 183fb932edbd..7f35b9880485 100755 --- a/tools/testing/selftests/gpio/gpio-mockup.sh +++ b/tools/testing/selftests/gpio/gpio-mockup.sh @@ -2,10 +2,11 @@ # SPDX-License-Identifier: GPL-2.0 #exit status -#1: run as non-root user +#1: Internal error #2: sysfs/debugfs not mount #3: insert module fail when gpio-mockup is a module. -#4: other reason. +#4: Skip test including run as non-root user. +#5: other reason. SYSFS= GPIO_SYSFS= @@ -15,6 +16,9 @@ GPIO_DEBUGFS= dev_type= module= +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + usage() { echo "Usage:" @@ -34,7 +38,7 @@ prerequisite() msg="skip all tests:" if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 1 + exit $ksft_skip fi SYSFS=`mount -t sysfs | head -1 | awk '{ print $3 }'` if [ ! -d "$SYSFS" ]; then @@ -73,7 +77,7 @@ remove_module() die() { remove_module - exit 4 + exit 5 } test_chips() -- 2.14.1
[PATCH 09/24] selftests: ftrace: return Kselftest Skip code for skipped tests
When ftrace test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. 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/ftrace/ftracetest | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/ftrace/ftracetest b/tools/testing/selftests/ftrace/ftracetest index f9a9d424c980..b731c8cdcffb 100755 --- a/tools/testing/selftests/ftrace/ftracetest +++ b/tools/testing/selftests/ftrace/ftracetest @@ -23,6 +23,9 @@ echo "If is -, all logs output in console only" exit $1 } +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + errexit() { # message echo "Error: $1" 1>&2 exit 1 @@ -30,7 +33,8 @@ errexit() { # message # Ensuring user privilege if [ `id -u` -ne 0 ]; then - errexit "this must be run by root user" + echo "Skipping: test must be run by root user" + exit $ksft_skip fi # Utilities @@ -249,7 +253,7 @@ trap 'SIG_RESULT=$UNTESTED' $SIG_UNTESTED SIG_UNSUPPORTED=$((SIG_BASE + UNSUPPORTED)) exit_unsupported () { kill -s $SIG_UNSUPPORTED $SIG_PID - exit 0 + exit $ksft_skip } trap 'SIG_RESULT=$UNSUPPORTED' $SIG_UNSUPPORTED -- 2.14.1
[PATCH 08/24] selftests: firmware: return Kselftest Skip code for skipped tests
When firmware test(s) get skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. 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/firmware/fw_fallback.sh | 4 ++-- tools/testing/selftests/firmware/fw_filesystem.sh | 4 +++- tools/testing/selftests/firmware/fw_lib.sh| 7 +-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/tools/testing/selftests/firmware/fw_fallback.sh b/tools/testing/selftests/firmware/fw_fallback.sh index 8e2e34a2ca69..70d18be46af5 100755 --- a/tools/testing/selftests/firmware/fw_fallback.sh +++ b/tools/testing/selftests/firmware/fw_fallback.sh @@ -74,7 +74,7 @@ load_fw_custom() { if [ ! -e "$DIR"/trigger_custom_fallback ]; then echo "$0: custom fallback trigger not present, ignoring test" >&2 - return 1 + exit $ksft_skip fi local name="$1" @@ -107,7 +107,7 @@ load_fw_custom_cancel() { if [ ! -e "$DIR"/trigger_custom_fallback ]; then echo "$0: canceling custom fallback trigger not present, ignoring test" >&2 - return 1 + exit $ksft_skip fi local name="$1" diff --git a/tools/testing/selftests/firmware/fw_filesystem.sh b/tools/testing/selftests/firmware/fw_filesystem.sh index 6452d2129cd9..a4320c4b44dc 100755 --- a/tools/testing/selftests/firmware/fw_filesystem.sh +++ b/tools/testing/selftests/firmware/fw_filesystem.sh @@ -30,6 +30,7 @@ fi if [ ! -e "$DIR"/trigger_async_request ]; then echo "$0: empty filename: async trigger not present, ignoring test" >&2 + exit $ksft_skip else if printf '\000' >"$DIR"/trigger_async_request 2> /dev/null; then echo "$0: empty filename should not succeed (async)" >&2 @@ -69,6 +70,7 @@ fi # Try the asynchronous version too if [ ! -e "$DIR"/trigger_async_request ]; then echo "$0: firmware loading: async trigger not present, ignoring test" >&2 + exit $ksft_skip else if ! echo -n "$NAME" >"$DIR"/trigger_async_request ; then echo "$0: could not trigger async request" >&2 @@ -89,7 +91,7 @@ test_config_present() { if [ ! -f $DIR/reset ]; then echo "Configuration triggers not present, ignoring test" - exit 0 + exit $ksft_skip fi } diff --git a/tools/testing/selftests/firmware/fw_lib.sh b/tools/testing/selftests/firmware/fw_lib.sh index 962d7f4ac627..6c5f1b2ffb74 100755 --- a/tools/testing/selftests/firmware/fw_lib.sh +++ b/tools/testing/selftests/firmware/fw_lib.sh @@ -9,11 +9,14 @@ DIR=/sys/devices/virtual/misc/test_firmware PROC_CONFIG="/proc/config.gz" TEST_DIR=$(dirname $0) +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + print_reqs_exit() { echo "You must have the following enabled in your kernel:" >&2 cat $TEST_DIR/config >&2 - exit 1 + exit $ksft_skip } test_modprobe() @@ -88,7 +91,7 @@ verify_reqs() if [ "$TEST_REQS_FW_SYSFS_FALLBACK" = "yes" ]; then if [ ! "$HAS_FW_LOADER_USER_HELPER" = "yes" ]; then echo "usermode helper disabled so ignoring test" - exit 0 + exit $ksft_skip fi fi } -- 2.14.1
[PATCH 07/24] selftests: filesystems: return Kselftest Skip code for skipped tests
When devpts_pts test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error which is treated as a fail by the Kselftest framework. This leads to false negative result even when the test could not be run. In another case, it returns pass for a skipped test reporting a false postive. Change it to return kselftest skip code when a test gets skipped to clearly report that the test could not be run. Change it to use ksft_exit_skip() when test is skipped. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/filesystems/devpts_pts.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/filesystems/devpts_pts.c b/tools/testing/selftests/filesystems/devpts_pts.c index b9055e974289..0f2d9f427944 100644 --- a/tools/testing/selftests/filesystems/devpts_pts.c +++ b/tools/testing/selftests/filesystems/devpts_pts.c @@ -11,6 +11,7 @@ #include #include #include +#include "../kselftest.h" static bool terminal_dup2(int duplicate, int original) { @@ -125,10 +126,12 @@ static int do_tiocgptpeer(char *ptmx, char *expected_procfd_contents) if (errno == EINVAL) { fprintf(stderr, "TIOCGPTPEER is not supported. " "Skipping test.\n"); - fret = EXIT_SUCCESS; + fret = KSFT_SKIP; + } else { + fprintf(stderr, + "Failed to perform TIOCGPTPEER ioctl\n"); + fret = EXIT_FAILURE; } - - fprintf(stderr, "Failed to perform TIOCGPTPEER ioctl\n"); goto do_cleanup; } @@ -281,7 +284,7 @@ int main(int argc, char *argv[]) if (!isatty(STDIN_FILENO)) { fprintf(stderr, "Standard input file desciptor is not attached " "to a terminal. Skipping test\n"); - exit(EXIT_FAILURE); + exit(KSFT_SKIP); } ret = unshare(CLONE_NEWNS); -- 2.14.1
[PATCH 06/24] selftests: exec: return Kselftest Skip code for skipped tests
When execveat test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when kernel doesn't support execveat. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/exec/execveat.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/exec/execveat.c b/tools/testing/selftests/exec/execveat.c index 67cd4597db2b..47cbf54d0801 100644 --- a/tools/testing/selftests/exec/execveat.c +++ b/tools/testing/selftests/exec/execveat.c @@ -20,6 +20,8 @@ #include #include +#include "../kselftest.h" + static char longpath[2 * PATH_MAX] = ""; static char *envp[] = { "IN_TEST=yes", NULL, NULL }; static char *argv[] = { "execveat", "99", NULL }; @@ -249,8 +251,8 @@ static int run_tests(void) errno = 0; execveat_(-1, NULL, NULL, NULL, 0); if (errno == ENOSYS) { - printf("[FAIL] ENOSYS calling execveat - no kernel support?\n"); - return 1; + ksft_exit_skip( + "ENOSYS calling execveat - no kernel support?\n"); } /* Change file position to confirm it doesn't affect anything */ -- 2.14.1
[PATCH 04/24] selftests: cpufreq: return Kselftest Skip code for skipped tests
When cpufreq test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. 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/cpufreq/main.sh | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/cpufreq/main.sh b/tools/testing/selftests/cpufreq/main.sh index d83922de9d89..31f8c9a76c5f 100755 --- a/tools/testing/selftests/cpufreq/main.sh +++ b/tools/testing/selftests/cpufreq/main.sh @@ -13,6 +13,9 @@ SYSFS= CPUROOT= CPUFREQROOT= +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + helpme() { printf "Usage: $0 [-h] [-todg args] @@ -38,7 +41,7 @@ prerequisite() if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 2 + exit $ksft_skip fi taskset -p 01 $$ -- 2.14.1
[PATCH 05/24] selftests: efivarfs: return Kselftest Skip code for skipped tests
When efivarfs test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. 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/efivarfs/efivarfs.sh | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh b/tools/testing/selftests/efivarfs/efivarfs.sh index c6d5790575ae..a47029a799d2 100755 --- a/tools/testing/selftests/efivarfs/efivarfs.sh +++ b/tools/testing/selftests/efivarfs/efivarfs.sh @@ -4,18 +4,21 @@ efivarfs_mount=/sys/firmware/efi/efivars test_guid=210be57c-9849-4fc7-a635-e6382d1aec27 +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + check_prereqs() { local msg="skip all tests:" if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi if ! grep -q "^\S\+ $efivarfs_mount efivarfs" /proc/mounts; then echo $msg efivarfs is not mounted on $efivarfs_mount >&2 - exit 0 + exit $ksft_skip fi } -- 2.14.1
[PATCH 03/24] selftests: cpu-hotplug: return Kselftest Skip code for skipped tests
When cpu-on-off-test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. 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/cpu-hotplug/cpu-on-off-test.sh | 14 -- 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh b/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh index f3a8933c1275..bab13dd025a6 100755 --- a/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh +++ b/tools/testing/selftests/cpu-hotplug/cpu-on-off-test.sh @@ -2,6 +2,8 @@ # SPDX-License-Identifier: GPL-2.0 SYSFS= +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 prerequisite() { @@ -9,7 +11,7 @@ prerequisite() if [ $UID != 0 ]; then echo $msg must be run as root >&2 - exit 0 + exit $ksft_skip fi taskset -p 01 $$ @@ -18,12 +20,12 @@ prerequisite() if [ ! -d "$SYSFS" ]; then echo $msg sysfs is not mounted >&2 - exit 0 + exit $ksft_skip fi if ! ls $SYSFS/devices/system/cpu/cpu* > /dev/null 2>&1; then echo $msg cpu hotplug is not supported >&2 - exit 0 + exit $ksft_skip fi echo "CPU online/offline summary:" @@ -32,7 +34,7 @@ prerequisite() if [[ "$online_cpus" = "$online_max" ]]; then echo "$msg: since there is only one cpu: $online_cpus" - exit 0 + exit $ksft_skip fi echo -e "\t Cpus in online state: $online_cpus" @@ -237,12 +239,12 @@ prerequisite_extra() if [ ! -d "$DEBUGFS" ]; then echo $msg debugfs is not mounted >&2 - exit 0 + exit $ksft_skip fi if [ ! -d $NOTIFIER_ERR_INJECT_DIR ]; then echo $msg cpu-notifier-error-inject module is not available >&2 - exit 0 + exit $ksft_skip fi } -- 2.14.1
[PATCH 01/24] selftests: android: ion: return Kselftest Skip code for skipped tests
When ion test is skipped because of unmet dependencies and/or unsupported configuration, it returns 0 which is treated as a pass by the Kselftest framework. This leads to false positive 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. 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/android/ion/ion_test.sh | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/testing/selftests/android/ion/ion_test.sh b/tools/testing/selftests/android/ion/ion_test.sh index a1aff506f5e6..69e676cfc94e 100755 --- a/tools/testing/selftests/android/ion/ion_test.sh +++ b/tools/testing/selftests/android/ion/ion_test.sh @@ -4,6 +4,9 @@ heapsize=4096 TCID="ion_test.sh" errcode=0 +# Kselftest framework requirement - SKIP code is 4. +ksft_skip=4 + run_test() { heaptype=$1 @@ -25,7 +28,7 @@ check_root() uid=$(id -u) if [ $uid -ne 0 ]; then echo $TCID: must be run as root >&2 - exit 0 + exit $ksft_skip fi } @@ -35,7 +38,7 @@ check_device() if [ ! -e $DEVICE ]; then echo $TCID: No $DEVICE device found >&2 echo $TCID: May be CONFIG_ION is not set >&2 - exit 0 + exit $ksft_skip fi } -- 2.14.1
[PATCH 02/24] selftests: breakpoints: return Kselftest Skip code for skipped tests
When step_after_suspend_test is skipped because of unmet dependencies and/or unsupported configuration, it exits with error 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. Change it to use ksft_exit_skip() when a non-root user runs the test and add an explicit check for root and a clear message, instead of failing the test when /sys/power/state file open fails. Signed-off-by: Shuah Khan (Samsung OSG) --- tools/testing/selftests/breakpoints/step_after_suspend_test.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/breakpoints/step_after_suspend_test.c b/tools/testing/selftests/breakpoints/step_after_suspend_test.c index 3fece06e9f64..f82dcc1f8841 100644 --- a/tools/testing/selftests/breakpoints/step_after_suspend_test.c +++ b/tools/testing/selftests/breakpoints/step_after_suspend_test.c @@ -143,10 +143,14 @@ void suspend(void) int err; struct itimerspec spec = {}; + if (getuid() != 0) + ksft_exit_skip("Please run the test as root - Exiting.\n"); + power_state_fd = open("/sys/power/state", O_RDWR); if (power_state_fd < 0) ksft_exit_fail_msg( - "open(\"/sys/power/state\") failed (is this test running as root?)\n"); + "open(\"/sys/power/state\") failed %s)\n", + strerror(errno)); timerfd = timerfd_create(CLOCK_BOOTTIME_ALARM, 0); if (timerfd < 0) -- 2.14.1
Re: [PATCH v13 3/3] mm, powerpc, x86: introduce an additional vma bit for powerpc pkey
On Fri, May 04, 2018 at 03:57:33PM -0700, Dave Hansen wrote: > > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > > index 0c9e392..3c7 100644 > > --- a/fs/proc/task_mmu.c > > +++ b/fs/proc/task_mmu.c > > @@ -679,6 +679,7 @@ static void show_smap_vma_flags(struct seq_file *m, > > struct vm_area_struct *vma) > > [ilog2(VM_PKEY_BIT1)] = "", > > [ilog2(VM_PKEY_BIT2)] = "", > > [ilog2(VM_PKEY_BIT3)] = "", > > + [ilog2(VM_PKEY_BIT4)] = "", > > #endif /* CONFIG_ARCH_HAS_PKEYS */ > ... > > +#if defined(CONFIG_PPC) > > +# define VM_PKEY_BIT4 VM_HIGH_ARCH_4 > > +#else > > +# define VM_PKEY_BIT4 0 > > +#endif > > #endif /* CONFIG_ARCH_HAS_PKEYS */ > > 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 -- Ram Pai
*alloc API changes
Hi, After writing up this email, I think I don't like this idea, but I'm still curious to see what people think of the general observations... 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. allocation method (not all available in all APIs): regular (kmalloc), zeroed (kzalloc), array (kmalloc_array), zeroed array (kcalloc) with or without node argument: +_node so, for example, we have things like: kmalloc_array_node() or pci_zalloc_consistent() Using some attempts at static analysis with git grep and coccinelle, nearly all of these take GFP flags, but something near 80% of functions are using GFP_KERNEL. Roughly half of the callers are including a sizeof(). Only 20% are using zeroing, 15% are using products as a size, and 3% are using ..._node, etc. 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. kmalloc()-family was meant to be a simplification of kmem_cache_alloc(). 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) However, then kzalloc(size, GFP_KERNEL) becomes the ugly: kmalloc_attrs(size, GFP_KERNEL, ALLOC_ZERO, NUMA_NO_NODE); (But I guess we could make macro wrappers for zeroing, or node?) 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) void *kmalloc(size_t size) { if (size == SIZE_MAX) return NULL; kmalloc_attrs(size, GFP_KERNEL, ALLOC_NOZERO, NUMA_NO_NODE); } 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. 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? Bleh. C sucks for this. -Kees -- Kees Cook Pixel Security
Re: [v2] mm: access to uninitialized struct page
Hi Pavel, FYI here is 0day's bisect result. The attached dmesg has reproduce script at the bottom. [27e2ce5dba4c30db031744c8140675d03d2ae7aa] mm: access to uninitialized struct page git://git.cmpxchg.org/linux-mmotm.git devel-catchup-201805041701 git bisect start 53eff77ad4b0adaf1ca6e1ecc6acf3804c344531 6da6c0db5316275015e8cc2959f12a17584aeb64 -- git bisect bad 3fc24705ffb48c18b23ce2c229f5018d39b18ab0 # 20:22 B 2Merge 'djwong-xfs/djwong-devel' into devel-catchup-201805041701 git bisect bad 78bd9ee71ffbbe5ab169cbe469503af2dfb913f9 # 20:35 B 2Merge 'linux-review/Geert-Uytterhoeven/dt-bindings-can-rcar_can-Fix-R8A7796-SoC-name/20180504-154952' into devel-catchup-201805041701 git bisect bad 1deba87932c5d0adcffe63d8ce4847f39e864775 # 20:56 B 2Merge 'yhuang/fix_thp_swap' into devel-catchup-201805041701 git bisect good e0997365e1e89e8bc9f5ed4a58a6cd2500b58668 # 21:09 G 20day base guard for 'devel-catchup-201805041701' git bisect bad 98815bfa9156a8d1da1f1ca5f3748e250fa19a88 # 21:22 B 2Merge 'yhuang/thp_delay_split3_r1a' into devel-catchup-201805041701 git bisect good 6da6c0db5316275015e8cc2959f12a17584aeb64 # 21:22 G 3Linux v4.17-rc3 git bisect bad 97c561bb48a33e135a90573c596bc755ed4eab32 # 21:36 B 2mm, pagemap: Hide swap entry for unprivileged users git bisect bad 466b08a3a87e8e43af677375a4cf8eb105f50007 # 21:47 B 2mm, swap: fix race between swapoff and some swap operations git bisect bad 44ea77b7788384a7c27a960ec5752de08e47882a # 21:58 B 2zram-introduce-zram-memory-tracking-fix git bisect bad 7a53abd52e920e8a2c16ea2cb81439e6e87a7ea4 # 22:06 B 2prctl: add PR_[GS]ET_PDEATHSIG_PROC git bisect good 1fda92fccc022924575edb98191f1ad2c0477c31 # 22:15 G 2z3fold-fix-reclaim-lock-ups-checkpatch-fixes git bisect bad 685cc80b632235416b72869247df7d6ae2816d61 # 22:30 B 2mm: migrate: fix double call of radix_tree_replace_slot() git bisect bad 27e2ce5dba4c30db031744c8140675d03d2ae7aa # 22:55 B 2mm: access to uninitialized struct page git bisect good 7a0e68e17b8aa41aa33e8c80015e36d47dde390a # 23:11 G 2mm: sections are not offlined during memory hotremove # extra tests on first bad commit # bad: [27e2ce5dba4c30db031744c8140675d03d2ae7aa] mm: access to uninitialized struct page # extra tests on parent commit # good: [7a0e68e17b8aa41aa33e8c80015e36d47dde390a] mm: sections are not offlined during memory hotremove tests: 2 testcase/path_params/tbox_group/run: boot/1/vm-vp-quantal-x86_64 7a0e68e17b8aa41a 27e2ce5dba4c30db031744c814 -- fail:runs %reproductionfail:runs | | | :2 100% 2:2 dmesg.BUG:kernel_reboot-without-warning_in_boot_stage testcase/path_params/tbox_group/run: boot/1/vm-lkp-nex04-yocto-x86_64 7a0e68e17b8aa41a 27e2ce5dba4c30db031744c814 -- :4 100% 4:4 dmesg.BUG:kernel_reboot-without-warning_in_boot_stage Thanks, Fengguang dmesg.xz Description: application/xz # # Automatically generated file; DO NOT EDIT. # Linux/x86_64 4.17.0-rc3 Kernel Configuration # CONFIG_64BIT=y CONFIG_X86_64=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf64-x86-64" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/x86_64_defconfig" CONFIG_LOCKDEP_SUPPORT=y CONFIG_STACKTRACE_SUPPORT=y CONFIG_MMU=y CONFIG_ARCH_MMAP_RND_BITS_MIN=28 CONFIG_ARCH_MMAP_RND_BITS_MAX=32 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MIN=8 CONFIG_ARCH_MMAP_RND_COMPAT_BITS_MAX=16 CONFIG_NEED_DMA_MAP_STATE=y CONFIG_NEED_SG_DMA_LENGTH=y CONFIG_GENERIC_ISA_DMA=y CONFIG_GENERIC_BUG=y CONFIG_GENERIC_BUG_RELATIVE_POINTERS=y CONFIG_GENERIC_HWEIGHT=y CONFIG_ARCH_MAY_HAVE_PC_FDC=y CONFIG_RWSEM_XCHGADD_ALGORITHM=y CONFIG_GENERIC_CALIBRATE_DELAY=y CONFIG_ARCH_HAS_CPU_RELAX=y CONFIG_ARCH_HAS_CACHE_LINE_SIZE=y CONFIG_ARCH_HAS_FILTER_PGPROT=y CONFIG_HAVE_SETUP_PER_CPU_AREA=y CONFIG_NEED_PER_CPU_EMBED_FIRST_CHUNK=y CONFIG_NEED_PER_CPU_PAGE_FIRST_CHUNK=y CONFIG_ARCH_HIBERNATION_POSSIBLE=y CONFIG_ARCH_SUSPEND_POSSIBLE=y CONFIG_ARCH_WANT_HUGE_PMD_SHARE=y CONFIG_ARCH_WANT_GENERAL_HUGETLB=y CONFIG_ZONE_DMA32=y CONFIG_AUDIT_ARCH=y CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING=y CONFIG_ARCH_SUPPORTS_DEBUG_PAGEALLOC=y CONFIG_ARCH_SUPPORTS_UPROBES=y CONFIG_FIX_EARLYCON_MEM=y CONFIG_PGTABLE_LEVELS=4 CONFIG_CONSTRUCTORS=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_CROSS_COMPILE="" # CONFIG_COMPILE_TEST is not set CONFIG_LOCALVERSION="" CONFIG_LOCALVERSION_AUTO=y CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y # CONFIG_KERNEL_GZIP is not set
Re: [PATCH v2 net-next 2/4] net: add skeleton of bpfilter kernel module
On Thu, May 03, 2018 at 03:23:55PM +0100, Edward Cree wrote: > On 03/05/18 05:36, Alexei Starovoitov wrote: > > bpfilter.ko consists of bpfilter_kern.c (normal kernel module code) > > and user mode helper code that is embedded into bpfilter.ko > > > > The steps to build bpfilter.ko are the following: > > - main.c is compiled by HOSTCC into the bpfilter_umh elf executable file > > - with quite a bit of objcopy and Makefile magic the bpfilter_umh elf file > > is converted into bpfilter_umh.o object file > > with _binary_net_bpfilter_bpfilter_umh_start and _end symbols > > Example: > > $ nm ./bld_x64/net/bpfilter/bpfilter_umh.o > > 4cf8 T _binary_net_bpfilter_bpfilter_umh_end > > 4cf8 A _binary_net_bpfilter_bpfilter_umh_size > > T _binary_net_bpfilter_bpfilter_umh_start > > - bpfilter_umh.o and bpfilter_kern.o are linked together into bpfilter.ko > > > > bpfilter_kern.c is a normal kernel module code that calls > > the fork_usermode_blob() helper to execute part of its own data > > as a user mode process. > > > > Notice that _binary_net_bpfilter_bpfilter_umh_start - end > > is placed into .init.rodata section, so it's freed as soon as __init > > function of bpfilter.ko is finished. > > As part of __init the bpfilter.ko does first request/reply action > > via two unix pipe provided by fork_usermode_blob() helper to > > make sure that umh is healthy. If not it will kill it via pid. > > > > Later bpfilter_process_sockopt() will be called from bpfilter hooks > > in get/setsockopt() to pass iptable commands into umh via bpfilter.ko > > > > If admin does 'rmmod bpfilter' the __exit code bpfilter.ko will > > kill umh as well. > > > > Signed-off-by: Alexei Starovoitov ... > > +static void stop_umh(void) > > +{ > > + if (bpfilter_process_sockopt) { > I worry about locking here. Is it possible for two calls to > bpfilter_process_sockopt() to run in parallel, both fail, and thus both > call stop_umh()? And if both end up calling shutdown_umh(), we double > fput(). I thought iptables sockopt is serialized earlier. Nope. We need to grab the mutex to access these pipes. Will fix. Thanks for spelling nits. Will fix as well.
Re: [PATCH 1/3] spi: meson-axg: support MAX 80M clock
On Fri, May 04, 2018 at 09:56:10AM +0800, Yixun Lan wrote: > Here I only introduce the dt compatible data to differentiate the > old/new controller, the compatible name is not changed, and none of the > property is introduced. Right, sorry - this bit is fine. signature.asc Description: PGP signature
Re: rcu-bh design
On Fri, May 4, 2018 at 4:42 PM Paul E. McKenney wrote: [..] > > Say that the tracepoint code is buggy and for some reason a > > call_srcu wasn't done. For example, say the hypothetical bug I'm > > taking about is in tracepoint_remove_func which called the > > rcu_assign_pointer, but didn't call release_probes: > > > > diff --git a/kernel/tracepoint.c b/kernel/tracepoint.c > > index d0639d917899..f54cb358f451 100644 > > --- a/kernel/tracepoint.c > > +++ b/kernel/tracepoint.c > > @@ -216,7 +216,6 @@ static int tracepoint_add_func(struct tracepoint *tp, > > rcu_assign_pointer(tp->funcs, tp_funcs); > > if (!static_key_enabled(&tp->key)) > > static_key_slow_inc(&tp->key); > > - release_probes(old); > > return 0; > > } > > --- > > I want to catching this from the test code. If I did the following, > > it would be insufficient then: > > > > trace_probe_register(...); > > srcu_barrier(&my_srcu_struct); // only tells me any queued calls > > // are done but not that something > > // was queued > > > > I was seeing if I could use my_srcu_struct::completed for that. but > > I'm not sure if that'll work (or that its legal to access it > > directly - but hey this is just test code! :P). > This is a bit ugly to do programmatically in the kernel. The callbacks > are on per-CPU queues that cannot be safely accessed except by that CPU. > Plus they are on singly linked lists that can be quite long, so it could > be insanely slow as well. > But given that this is test code, why not leverage event tracing? > There are some relevant events in include/trace/events/rcu.h, starting > with the rcu_callback trace event. Sounds like a great idea. I'll try that, thanks. - Joel
Re: Oops on the system startup in the function part_in_flight()
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 Thanks, Ben -- Ben Greear Candela Technologies Inc http://www.candelatech.com
Re: [PATCH] elf: don't hash userspace addresses
Alexey Dobriyan wrote: > Signed-off-by: Alexey Dobriyan > --- > > fs/binfmt_elf.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -379,8 +379,8 @@ static unsigned long elf_map(struct file *filep, unsigned > long addr, > > if ((type & MAP_FIXED_NOREPLACE) && > PTR_ERR((void *)map_addr) == -EEXIST) > - pr_info("%d (%s): Uhuuh, elf segment at %px requested but the > memory is mapped already\n", > - task_pid_nr(current), current->comm, (void *)addr); > + pr_info("%d (%s): Uhuuh, elf segment at %lx requested but the > memory is mapped already\n", > + task_pid_nr(current), current->comm, addr); "%px" does not hash. For what reason you replace %px with %lx ? > > return(map_addr); > } >
[GIT PULL] remoteproc and rpmsg fixes for v4.17
The following changes since commit 60cc43fc888428bb2f18f08997432d426a243338: Linux 4.17-rc1 (2018-04-15 18:24:20 -0700) are available in the Git repository at: git://github.com/andersson/remoteproc tags/rproc-v4.17-1 for you to fetch changes up to 93dd4e73c0d9cc32f835d76a54257020b0bfc75a: rpmsg: added MODULE_ALIAS for rpmsg_char (2018-04-25 16:46:55 -0700) remoteproc and rpmsg fixes for v4.17 Fixes screw up when reversing boolean for rproc_stop(), add missing of node dereferences and add missing MODULE_ALIAS in rpmsg_char. Arnaud Pouliquen (1): remoteproc: fix crashed parameter logic on stop call Ramon Fried (1): rpmsg: added MODULE_ALIAS for rpmsg_char Tobias Jordan (1): remoteproc: qcom: Fix potential device node leaks drivers/remoteproc/qcom_q6v5_pil.c | 2 ++ drivers/remoteproc/remoteproc_core.c | 4 ++-- drivers/rpmsg/rpmsg_char.c | 2 ++ include/linux/remoteproc.h | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-)
Re: [PATCH V3] Input: pm8941-pwrkey: add resin key capabilities
Hi Tirupathi, On Fri, Mar 23, 2018 at 11:53:12AM +0530, Tirupathi Reddy wrote: > Add resin key support to handle different types of key events > defined in different platforms. > > Signed-off-by: Tirupathi Reddy > --- > .../bindings/input/qcom,pm8941-pwrkey.txt | 32 + > drivers/input/misc/pm8941-pwrkey.c | 81 > ++ > 2 files changed, 113 insertions(+) > > diff --git a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt > b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt > index 07bf55f..c671636 100644 > --- a/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt > +++ b/Documentation/devicetree/bindings/input/qcom,pm8941-pwrkey.txt > @@ -32,6 +32,32 @@ PROPERTIES > Definition: presence of this property indicates that the KPDPWR_N pin > should be configured for pull up. > > +RESIN SUBNODE > + > +The HW module can generate other optional key events like RESIN(reset-in > pin). > +The RESIN pin can be configured to support different key events on different > +platforms. The resin key is described by the following properties. > + > +- interrupts: > + Usage: required > + Value type: > + Definition: key change interrupt; The format of the specifier is > + defined by the binding document describing the node's > + interrupt parent. > + > +- linux,code: > + Usage: required > + Value type: > + Definition: The input key-code associated with the resin key. > + Use the linux event codes defined in > + include/dt-bindings/input/linux-event-codes.h > + > +- bias-pull-up: > + Usage: optional > + Value type: > + Definition: presence of this property indicates that the RESIN pin > + should be configured for pull up. > + > EXAMPLE > > pwrkey@800 { > @@ -40,4 +66,10 @@ EXAMPLE > interrupts = <0x0 0x8 0 IRQ_TYPE_EDGE_BOTH>; > debounce = <15625>; > bias-pull-up; > + > + resin { > + interrupts = <0x0 0x8 1 IRQ_TYPE_EDGE_BOTH>; > + linux,code = ; > + bias-pull-up; > + }; > }; The new key and power key bindings are very similar, I would prefer if we shared the parsing code and our new DTS looked like: power { ... }; resin { ... }; (we can easily keep backward compatibility with power properties being in device node). > diff --git a/drivers/input/misc/pm8941-pwrkey.c > b/drivers/input/misc/pm8941-pwrkey.c > index 18ad956..6e45d01 100644 > --- a/drivers/input/misc/pm8941-pwrkey.c > +++ b/drivers/input/misc/pm8941-pwrkey.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -28,6 +29,7 @@ > > #define PON_RT_STS 0x10 > #define PON_KPDPWR_N_SETBIT(0) > +#define PON_RESIN_N_SET BIT(1) > > #define PON_PS_HOLD_RST_CTL 0x5a > #define PON_PS_HOLD_RST_CTL2 0x5b > @@ -37,6 +39,7 @@ > #define PON_PS_HOLD_TYPE_HARD_RESET 7 > > #define PON_PULL_CTL 0x70 > +#define PON_RESIN_PULL_UP BIT(0) > #define PON_KPDPWR_PULL_UP BIT(1) > > #define PON_DBC_CTL 0x71 > @@ -46,6 +49,7 @@ > struct pm8941_pwrkey { > struct device *dev; > int irq; > + u32 resin_key_code; > u32 baseaddr; > struct regmap *regmap; > struct input_dev *input; > @@ -130,6 +134,24 @@ static irqreturn_t pm8941_pwrkey_irq(int irq, void > *_data) > return IRQ_HANDLED; > } > > +static irqreturn_t pm8941_resinkey_irq(int irq, void *_data) > +{ > + struct pm8941_pwrkey *pwrkey = _data; > + unsigned int sts; > + int error; > + u32 key_code = pwrkey->resin_key_code; > + > + error = regmap_read(pwrkey->regmap, > + pwrkey->baseaddr + PON_RT_STS, &sts); > + if (error) > + return IRQ_HANDLED; > + > + input_report_key(pwrkey->input, key_code, !!(sts & PON_RESIN_N_SET)); > + input_sync(pwrkey->input); > + > + return IRQ_HANDLED; > +} > + > static int __maybe_unused pm8941_pwrkey_suspend(struct device *dev) > { > struct pm8941_pwrkey *pwrkey = dev_get_drvdata(dev); > @@ -153,9 +175,56 @@ static int __maybe_unused pm8941_pwrkey_resume(struct > device *dev) > static SIMPLE_DEV_PM_OPS(pm8941_pwr_key_pm_ops, >pm8941_pwrkey_suspend, pm8941_pwrkey_resume); > > +static int pm8941_resin_key_init(struct pm8941_pwrkey *pwrkey, > + struct device_node *np) > +{ > + int error, irq; > + bool pull_up; > + > + /* > + * Get the standard-key parameters. This might not be > + * specified if there is no key mapping on the reset line. > + */ >
Re: [PATCH V2 2/5] X86: Hyper-V: Enable IPI enlightenments
Hi Srinivasan, I love your patch! Yet something to improve: [auto build test ERROR on v4.17-rc3] [also build test ERROR on next-20180504] [cannot apply to tip/x86/core] [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/kys-linuxonhyperv-com/X86-Hyper-V-APIC-enlightenments/20180505-045627 config: x86_64-randconfig-a0-05050447 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): arch/x86/hyperv/hv_apic.c: In function 'hv_apic_read': arch/x86/hyperv/hv_apic.c:69:10: error: implicit declaration of function 'native_apic_mem_read'; did you mean 'hv_apic_icr_read'? [-Werror=implicit-function-declaration] return native_apic_mem_read(reg); ^~~~ hv_apic_icr_read arch/x86/hyperv/hv_apic.c: In function 'hv_apic_write': arch/x86/hyperv/hv_apic.c:83:3: error: implicit declaration of function 'native_apic_mem_write'; did you mean 'hv_apic_icr_write'? [-Werror=implicit-function-declaration] native_apic_mem_write(reg, val); ^ hv_apic_icr_write arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi': >> arch/x86/hyperv/hv_apic.c:153:12: error: invalid use of undefined type >> 'struct apic' orig_apic.send_IPI(cpu, vector); ^ arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask': arch/x86/hyperv/hv_apic.c:159:12: error: invalid use of undefined type 'struct apic' orig_apic.send_IPI_mask(mask, vector); ^ arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_mask_allbutself': arch/x86/hyperv/hv_apic.c:172:12: error: invalid use of undefined type 'struct apic' orig_apic.send_IPI_mask_allbutself(mask, vector); ^ arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_all': arch/x86/hyperv/hv_apic.c:183:12: error: invalid use of undefined type 'struct apic' orig_apic.send_IPI_all(vector); ^ arch/x86/hyperv/hv_apic.c: In function 'hv_send_ipi_self': arch/x86/hyperv/hv_apic.c:189:12: error: invalid use of undefined type 'struct apic' orig_apic.send_IPI_self(vector); ^ arch/x86/hyperv/hv_apic.c: In function 'hv_apic_init': >> arch/x86/hyperv/hv_apic.c:199:16: error: 'apic' undeclared (first use in >> this function) orig_apic = *apic; ^~~~ arch/x86/hyperv/hv_apic.c:199:16: note: each undeclared identifier is reported only once for each function it appears in >> arch/x86/hyperv/hv_apic.c:199:13: error: 'orig_apic' has an incomplete type >> 'struct apic' orig_apic = *apic; ^ arch/x86/hyperv/hv_apic.c:211:3: error: implicit declaration of function 'apic_set_eoi_write'; did you mean 'hv_apic_eoi_write'? [-Werror=implicit-function-declaration] apic_set_eoi_write(hv_apic_eoi_write); ^~ hv_apic_eoi_write arch/x86/hyperv/hv_apic.c: At top level: >> arch/x86/hyperv/hv_apic.c:35:20: error: storage size of 'orig_apic' isn't >> known static struct apic orig_apic; ^ cc1: some warnings being treated as errors vim +153 arch/x86/hyperv/hv_apic.c 34 > 35 static struct apic orig_apic; 36 37 static u64 hv_apic_icr_read(void) 38 { 39 u64 reg_val; 40 41 rdmsrl(HV_X64_MSR_ICR, reg_val); 42 return reg_val; 43 } 44 45 static void hv_apic_icr_write(u32 low, u32 id) 46 { 47 u64 reg_val; 48 49 reg_val = SET_APIC_DEST_FIELD(id); 50 reg_val = reg_val << 32; 51 reg_val |= low; 52 53 wrmsrl(HV_X64_MSR_ICR, reg_val); 54 } 55 56 static u32 hv_apic_read(u32 reg) 57 { 58 u32 reg_val, hi; 59 60 switch (reg) { 61 case APIC_EOI: 62 rdmsr(HV_X64_MSR_EOI, reg_val, hi); 63 return reg_val; 64 case APIC_TASKPRI: 65 rdmsr(HV_X64_MSR_TPR, reg_val, hi); 66 return reg_val; 67 68 default: 69 return native_apic_mem_read(reg); 70 } 71 } 72 73 static void hv_apic_write(u32 reg, u32 val) 74 { 75 switch (reg) { 76 case APIC_EOI: 77 wrmsr(HV_X64_MSR_EOI, val, 0); 78 break; 79 case APIC_TASKPRI: 80
[PATCH] proc: test /proc/*/fd a bit (+ PF_KTHREAD is ABI!)
* Test lookup in /proc/self/fd. "map_files" lookup story showed that lookup is not that simple. * Test that all those symlinks open the same file. Check with (st_dev, st_info). * Test that kernel threads do not have anything in their /proc/*/fd/ directory. Now this is where things get interesting. First, kernel threads aren't pinned by /proc/self or equivalent, thus some "atomicity" is required. Second, ->comm can contain whitespace and ')'. No, they are not escaped. Third, the only reliable way to check if process is kernel thread appears to be field #9 in /proc/*/stat. This field is struct task_struct::flags in decimal! Check is done by testing PF_KTHREAD flags like we do in kernel. PF_KTREAD value is a part of userspace ABI !!! Other methods for determining kernel threadness are not reliable: * RSS can be 0 if everything is swapped, even while reading from /proc/self. * ->total_vm CAN BE ZERO if process is finishing munmap(NULL, whole address space); * /proc/*/maps and similar files can be empty because unmapping everything works. Read returning 0 can't distinguish between kernel thread and such suicide process. Signed-off-by: Alexey Dobriyan --- tools/testing/selftests/proc/.gitignore|3 tools/testing/selftests/proc/Makefile |5 tools/testing/selftests/proc/fd-001-lookup.c | 168 +++ tools/testing/selftests/proc/fd-002-posix-eq.c | 57 tools/testing/selftests/proc/fd-003-kthread.c | 178 + tools/testing/selftests/proc/proc-uptime.h | 16 -- tools/testing/selftests/proc/proc.h| 39 + tools/testing/selftests/proc/read.c| 17 -- 8 files changed, 451 insertions(+), 32 deletions(-) --- a/tools/testing/selftests/proc/.gitignore +++ b/tools/testing/selftests/proc/.gitignore @@ -1,3 +1,6 @@ +/fd-001-lookup +/fd-002-posix-eq +/fd-003-kthread /proc-loadavg-001 /proc-self-map-files-001 /proc-self-map-files-002 --- a/tools/testing/selftests/proc/Makefile +++ b/tools/testing/selftests/proc/Makefile @@ -1,6 +1,9 @@ -CFLAGS += -Wall -O2 +CFLAGS += -Wall -O2 -Wno-unused-function TEST_GEN_PROGS := +TEST_GEN_PROGS += fd-001-lookup +TEST_GEN_PROGS += fd-002-posix-eq +TEST_GEN_PROGS += fd-003-kthread TEST_GEN_PROGS += proc-loadavg-001 TEST_GEN_PROGS += proc-self-map-files-001 TEST_GEN_PROGS += proc-self-map-files-002 new file mode 100644 --- /dev/null +++ b/tools/testing/selftests/proc/fd-001-lookup.c @@ -0,0 +1,168 @@ +/* + * Copyright © 2018 Alexey Dobriyan + * + * Permission to use, copy, modify, and distribute this software for any + * purpose with or without fee is hereby granted, provided that the above + * copyright notice and this permission notice appear in all copies. + * + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. + */ +// Test /proc/*/fd lookup. +#define _GNU_SOURCE +#undef NDEBUG +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "proc.h" + +/* lstat(2) has more "coverage" in case non-symlink pops up somehow. */ +static void test_lookup_pass(const char *pathname) +{ + struct stat st; + ssize_t rv; + + memset(&st, 0, sizeof(struct stat)); + rv = lstat(pathname, &st); + assert(rv == 0); + assert(S_ISLNK(st.st_mode)); +} + +static void test_lookup_fail(const char *pathname) +{ + struct stat st; + ssize_t rv; + + rv = lstat(pathname, &st); + assert(rv == -1 && errno == ENOENT); +} + +static void test_lookup(unsigned int fd) +{ + char buf[64]; + unsigned int c; + unsigned int u; + int i; + + snprintf(buf, sizeof(buf), "/proc/self/fd/%u", fd); + test_lookup_pass(buf); + + /* leading junk */ + for (c = 1; c <= 255; c++) { + if (c == '/') + continue; + snprintf(buf, sizeof(buf), "/proc/self/fd/%c%u", c, fd); + test_lookup_fail(buf); + } + + /* trailing junk */ + for (c = 1; c <= 255; c++) { + if (c == '/') + continue; + snprintf(buf, sizeof(buf), "/proc/self/fd/%u%c", fd, c); + test_lookup_fail(buf); + } + + for (i = INT_MIN; i < INT_MIN + 1024; i++) { + snprintf(buf, sizeof(buf), "/proc/self/fd/%d", i); + test_lookup_fail(buf); + } + for (i = -1024; i < 0; i++) { + snprintf(buf, sizeof(buf), "/p
Re: [PATCH v3 3/3] sh: add the sh_ prefix to early platform symbols
On Fri, May 4, 2018 at 9:27 AM, Bartosz Golaszewski wrote: > From: Bartosz Golaszewski > > Old early platform device support is now sh-specific. Before moving on > to implementing new early platform framework based on real platform > devices, prefix all early platform symbols with 'sh_'. > > Signed-off-by: Bartosz Golaszewski Looks good. I was thinking of something shorter as the new namespace, but actually using a longer name like you do seems appropriate since we don't want anyone to actually use it. ;-) Arnd