答复: [PATCH v5] docs/zh_CN: add translations in zh_CN/dev-tools/gcov

[Zengtao (B)]

> -邮件原件-
> 发件人: Wu XiangCheng [mailto:bob...@email.cn]
> 发送时间: 2021年4月14日 21:21
> 收件人: Alex Shi ; Bernard Zhao 
> 抄送: Jonathan Corbet ; YanTeng Si
> ; Nathan Chancellor ; Nick
> Desaulniers ; linux-...@vger.kernel.org;
> linux-kernel@vger.kernel.org; clang-built-li...@googlegroups.com
> 主题: [PATCH v5] docs/zh_CN: add translations in zh_CN/dev-tools/gcov
> 
> From: Bernard Zhao 
> 
> Add new zh translations
> * zh_CN/dev-tools/gcov.rst
> * zh_CN/dev-tools/index.rst
> and link them to zh_CN/index.rst
> 
> Signed-off-by: Bernard Zhao 
> Reviewed-by: Wu XiangCheng 
> Signed-off-by: Wu XiangCheng 
> ---
> base: linux-next
> commit 269dd42f4776 ("docs/zh_CN: add riscv to zh_CN index")
> 
> Changes since V4:
> * modified some words under Alex Shi's advices
> 
> Changes since V3:
> * update to newest linux-next
> * fix ``
> * fix tags
> * fix list indent
> 
> Changes since V2:
> * fix some inaccurate translation
> 
> Changes since V1:
> * add index.rst in dev-tools and link to to zh_CN/index.rst
> * fix some inaccurate translation
> 
>  .../translations/zh_CN/dev-tools/gcov.rst | 265 ++
>  .../translations/zh_CN/dev-tools/index.rst|  35 +++
>  Documentation/translations/zh_CN/index.rst|   1 +
>  3 files changed, 301 insertions(+)
>  create mode 100644 Documentation/translations/zh_CN/dev-tools/gcov.rst
>  create mode 100644 Documentation/translations/zh_CN/dev-tools/index.rst
> 
> diff --git a/Documentation/translations/zh_CN/dev-tools/gcov.rst
> b/Documentation/translations/zh_CN/dev-tools/gcov.rst
> new file mode 100644
> index ..7515b488bc4e
> --- /dev/null
> +++ b/Documentation/translations/zh_CN/dev-tools/gcov.rst
> @@ -0,0 +1,265 @@
> +.. include:: ../disclaimer-zh_CN.rst
> +
> +:Original: Documentation/dev-tools/gcov.rst
> +:Translator: 赵军奎 Bernard Zhao 
> +
> +在Linux内核里使用gcov做代码覆盖率检查
> +=
> +
> +gcov是linux中已经集成的一个分析模块，该模块在内核中对GCC的代码
> 覆盖率统

Gcov is a tool/function, misleading for " gcov是linux中已经集成的一个分析
模块"

I 'd suggest:
"Linux内核中已经集成一个特性支持gcov功能，该特性让用户可以使用gcov
 工具对内核代码覆盖率进行统计"

Thanks.

答复: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device

[Zengtao (B)]

> -邮件原件-
> 发件人: Alex Williamson [mailto:alex.william...@redhat.com]
> 发送时间: 2021年4月9日 22:24
> 收件人: Zengtao (B) 
> 抄送: coh...@redhat.com; k...@vger.kernel.org;
> linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com
> 主题: Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> 
> On Fri, 9 Apr 2021 04:54:23 +
> "Zengtao (B)"  wrote:
> 
> > > -邮件原件-
> > > 发件人: Alex Williamson [mailto:alex.william...@redhat.com]
> > > 发送时间: 2021年3月9日 5:47
> > > 收件人: alex.william...@redhat.com
> > > 抄送: coh...@redhat.com; k...@vger.kernel.org;
> > > linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com
> > > 主题: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> > >
> > > By linking all the device fds we provide to userspace to an address
> > > space through a new pseudo fs, we can use tools like
> > > unmap_mapping_range() to zap all vmas associated with a device.
> > >
> > > Suggested-by: Jason Gunthorpe 
> > > Signed-off-by: Alex Williamson 
> > > ---
> > >  drivers/vfio/vfio.c |   54
> > > +++
> > >  1 file changed, 54 insertions(+)
> > >
> > > diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> > > 38779e6fd80c..abdf8d52a911 100644
> > > --- a/drivers/vfio/vfio.c
> > > +++ b/drivers/vfio/vfio.c
> > > @@ -32,11 +32,18 @@
> > >  #include 
> > >  #include 
> > >  #include 
> > > +#include 
> > > +#include 
> > Minor: keep the headers in alphabetical order.
> 
> They started out that way, but various tree-wide changes ignoring that, and
> likely oversights on my part as well, has left us with numerous breaks in that
> rule already.
> 
> > >
> > >  #define DRIVER_VERSION   "0.3"
> > >  #define DRIVER_AUTHOR"Alex Williamson
> "
> > >  #define DRIVER_DESC  "VFIO - User Level meta-driver"
> > >
> > > +#define VFIO_MAGIC 0x5646494f /* "VFIO" */
> > Move to include/uapi/linux/magic.h ?
> 
> Hmm, yeah, I suppose it probably should go there.  Thanks.
> 
> FWIW, I'm still working on a next version of this series, currently struggling
> how to handle an arbitrary number of vmas per user DMA mapping.  Thanks,
> 

Will do some testing on our platform, and want to make sure the issue
 I reported is included : 
 
https://patchwork.kernel.org/project/kvm/patch/1615201890-887-1-git-send-email-prime.z...@hisilicon.com/
 
 Thanks
 zengtao


> Alex

答复: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device

[Zengtao (B)]

> -邮件原件-
> 发件人: Jason Gunthorpe [mailto:j...@nvidia.com]
> 发送时间: 2021年4月10日 1:32
> 收件人: Alex Williamson 
> 抄送: Zengtao (B) ; coh...@redhat.com;
> k...@vger.kernel.org; linux-kernel@vger.kernel.org; pet...@redhat.com
> 主题: Re: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> 
> On Fri, Apr 09, 2021 at 08:24:00AM -0600, Alex Williamson wrote:
> 
> > > >  #define DRIVER_VERSION "0.3"
> > > >  #define DRIVER_AUTHOR  "Alex Williamson
> "
> > > >  #define DRIVER_DESC"VFIO - User Level meta-driver"
> > > >
> > > > +#define VFIO_MAGIC 0x5646494f /* "VFIO" */
> > > Move to include/uapi/linux/magic.h ?
> >
> > Hmm, yeah, I suppose it probably should go there.  Thanks.
> 
> From my review we haven't been doing that unless it is actually uapi relavent
> for some reason (this is not)
>
Yes, it's not uapi relavent, and I still think it's better to put magic
 in a single header, and currently not all micros in magic.h are for
 uapi, some work should be done for that, but no ideas now, :)
 
> In particular with CH having a patch series to eliminate all of these cases 
> and
> drop the constants..
> 
Interested, could you share the links for that?

Thanks 
Zengtao 

> Jason

答复: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device

[Zengtao (B)]

> -邮件原件-
> 发件人: Alex Williamson [mailto:alex.william...@redhat.com]
> 发送时间: 2021年3月9日 5:47
> 收件人: alex.william...@redhat.com
> 抄送: coh...@redhat.com; k...@vger.kernel.org;
> linux-kernel@vger.kernel.org; j...@nvidia.com; pet...@redhat.com
> 主题: [PATCH v1 01/14] vfio: Create vfio_fs_type with inode per device
> 
> By linking all the device fds we provide to userspace to an address space
> through a new pseudo fs, we can use tools like
> unmap_mapping_range() to zap all vmas associated with a device.
> 
> Suggested-by: Jason Gunthorpe 
> Signed-off-by: Alex Williamson 
> ---
>  drivers/vfio/vfio.c |   54
> +++
>  1 file changed, 54 insertions(+)
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index
> 38779e6fd80c..abdf8d52a911 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -32,11 +32,18 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
Minor: keep the headers in alphabetical order.

> 
>  #define DRIVER_VERSION   "0.3"
>  #define DRIVER_AUTHOR"Alex Williamson "
>  #define DRIVER_DESC  "VFIO - User Level meta-driver"
> 
> +#define VFIO_MAGIC 0x5646494f /* "VFIO" */
Move to include/uapi/linux/magic.h ? 

> +
> +static int vfio_fs_cnt;
> +static struct vfsmount *vfio_fs_mnt;
> +
>  static struct vfio {
>   struct class*class;
>   struct list_headiommu_drivers_list;
> @@ -97,6 +104,7 @@ struct vfio_device {
>   struct vfio_group   *group;
>   struct list_headgroup_next;
>   void*device_data;
> + struct inode*inode;
>  };
> 
>  #ifdef CONFIG_VFIO_NOIOMMU
> @@ -529,6 +537,34 @@ static struct vfio_group
> *vfio_group_get_from_dev(struct device *dev)
>   return group;
>  }
> 
> +static int vfio_fs_init_fs_context(struct fs_context *fc) {
> + return init_pseudo(fc, VFIO_MAGIC) ? 0 : -ENOMEM; }
> +
> +static struct file_system_type vfio_fs_type = {
> + .name = "vfio",
> + .owner = THIS_MODULE,
> + .init_fs_context = vfio_fs_init_fs_context,
> + .kill_sb = kill_anon_super,
> +};
> +
> +static struct inode *vfio_fs_inode_new(void) {
> + struct inode *inode;
> + int ret;
> +
> + ret = simple_pin_fs(_fs_type, _fs_mnt, _fs_cnt);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + inode = alloc_anon_inode(vfio_fs_mnt->mnt_sb);
> + if (IS_ERR(inode))
> + simple_release_fs(_fs_mnt, _fs_cnt);
> +
> + return inode;
> +}
> +
>  /**
>   * Device objects - create, release, get, put, search
>   */
> @@ -539,11 +575,19 @@ struct vfio_device
> *vfio_group_create_device(struct vfio_group *group,
>void *device_data)
>  {
>   struct vfio_device *device;
> + struct inode *inode;
> 
>   device = kzalloc(sizeof(*device), GFP_KERNEL);
>   if (!device)
>   return ERR_PTR(-ENOMEM);
> 
> + inode = vfio_fs_inode_new();
> + if (IS_ERR(inode)) {
> + kfree(device);
> + return ERR_CAST(inode);
> + }
> + device->inode = inode;
> +
>   kref_init(>kref);
>   device->dev = dev;
>   device->group = group;
> @@ -574,6 +618,9 @@ static void vfio_device_release(struct kref *kref)
> 
>   dev_set_drvdata(device->dev, NULL);
> 
> + iput(device->inode);
> + simple_release_fs(_fs_mnt, _fs_cnt);
> +
>   kfree(device);
> 
>   /* vfio_del_group_dev may be waiting for this device */ @@ -1488,6
> +1535,13 @@ static int vfio_group_get_device_fd(struct vfio_group *group,
> char *buf)
>*/
>   filep->f_mode |= (FMODE_LSEEK | FMODE_PREAD | FMODE_PWRITE);
> 
> + /*
> +  * Use the pseudo fs inode on the device to link all mmaps
> +  * to the same address space, allowing us to unmap all vmas
> +  * associated to this device using unmap_mapping_range().
> +  */
> + filep->f_mapping = device->inode->i_mapping;
> +
>   atomic_inc(>container_users);
> 
>   fd_install(ret, filep);

答复: [PATCH] usb: dwc2: Using tab instead of blank

[Zengtao (B)]

Hi Minas, greg k-h

Please ignore this patch, it's an incomplete version sent by mistake,
 Sorry for the noise.

> -邮件原件-
> 发件人: Greg Kroah-Hartman [mailto:gre...@linuxfoundation.org]
> 发送时间: 2021年3月19日 17:32
> 收件人: Zengtao (B) 
> 抄送: hmi...@synopsys.com; Linuxarm ;
> linux-...@vger.kernel.org; linux-kernel@vger.kernel.org
> 主题: Re: [PATCH] usb: dwc2: Using tab instead of blank
> 
> On Fri, Mar 19, 2021 at 05:00:55PM +0800, Zeng Tao wrote:
> > Signed-off-by: Zeng Tao 
> 
> I can not take patches without any changelog text, sorry.
> 
> greg k-h

答复: [PATCH] vfio/pci: make the vfio_pci_mmap_fault reentrant

[Zengtao (B)]

Hi Alex:

> -邮件原件-
> 发件人: Alex Williamson [mailto:alex.william...@redhat.com]
> 发送时间: 2021年3月10日 14:09
> 收件人: Jason Gunthorpe 
> 抄送: Peter Xu ; Zengtao (B) ;
> Cornelia Huck ; Kevin Tian ;
> Andrew Morton ; Giovanni Cabiddu
> ; Michel Lespinasse ; Jann
> Horn ; Max Gurtovoy ;
> k...@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
> 
> 主题: Re: [PATCH] vfio/pci: make the vfio_pci_mmap_fault reentrant
> 
> On Tue, 9 Mar 2021 19:41:27 -0400
> Jason Gunthorpe  wrote:
> 
> > On Tue, Mar 09, 2021 at 12:26:07PM -0700, Alex Williamson wrote:
> >
> > > In the new series, I think the fault handler becomes (untested):
> > >
> > > static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf) {
> > > struct vm_area_struct *vma = vmf->vma;
> > > struct vfio_pci_device *vdev = vma->vm_private_data;
> > > unsigned long base_pfn, pgoff;
> > > vm_fault_t ret = VM_FAULT_SIGBUS;
> > >
> > > if (vfio_pci_bar_vma_to_pfn(vma, _pfn))
> > > return ret;
> > >
> > > pgoff = (vmf->address - vma->vm_start) >> PAGE_SHIFT;
> >
> > I don't think this math is completely safe, it needs to parse the
> > vm_pgoff..
> >
> > I'm worried userspace could split/punch/mangle a VMA using
> > munmap/mremap/etc/etc in a way that does update the pg_off but is
> > incompatible with the above.
> 
> parsing vm_pgoff is done in:
> 
> static int vfio_pci_bar_vma_to_pfn(struct vm_area_struct *vma,
>unsigned long *pfn) {
> struct vfio_pci_device *vdev = vma->vm_private_data;
> struct pci_dev *pdev = vdev->pdev;
> int index;
> u64 pgoff;
> 
> index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT);
> 
> if (index >= VFIO_PCI_ROM_REGION_INDEX ||
> !vdev->bar_mmap_supported[index] || !vdev->barmap[index])
> return -EINVAL;
> 
> pgoff = vma->vm_pgoff &
> ((1U << (VFIO_PCI_OFFSET_SHIFT - PAGE_SHIFT)) - 1);
> 
> *pfn = (pci_resource_start(pdev, index) >> PAGE_SHIFT) + pgoff;
> 
> return 0;
> }
> 
> But given Peter's concern about faulting individual pages, I think the fault 
> handler
> becomes:
> 
> static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf) {
> struct vm_area_struct *vma = vmf->vma;
> struct vfio_pci_device *vdev = vma->vm_private_data;
> unsigned long vaddr, pfn;
> vm_fault_t ret = VM_FAULT_SIGBUS;
> 
> if (vfio_pci_bar_vma_to_pfn(vma, ))
> return ret;
> 
> down_read(>memory_lock);
> 
> if (__vfio_pci_memory_enabled(vdev)) {
> for (vaddr = vma->vm_start;
>  vaddr < vma->vm_end; vaddr += PAGE_SIZE, pfn++) {
One concern here is the performance, since you are doing the mapping for the
 whole vma, what about using block mapping if applicable?

> ret = vmf_insert_pfn_prot(vma, vaddr, pfn,
> 
> pgprot_decrypted(vma->vm_page_prot));
> if (ret != VM_FAULT_NOPAGE) {
> zap_vma_ptes(vma, vma->vm_start,
>  vaddr - vma->vm_start);
> break;
> }
> }
> }
> 
> up_read(>memory_lock);
> 
> return ret;
> }
> 
> Thanks,
> Alex

答复: [PATCH] vfio/pci: make the vfio_pci_mmap_fault reentrant

[Zengtao (B)]

Hi guys:

Thanks for the helpful comments, after rethinking the issue, I have proposed
 the following change: 
1. follow_pte instead of follow_pfn.
2. vmf_insert_pfn loops instead of io_remap_pfn_range
3. proper undos when some call fails.
4. keep the bigger lock range to avoid unessary pte install. 

please help to take a look and get your comments, thanks.

static vm_fault_t vfio_pci_mmap_fault(struct vm_fault *vmf)
{
struct vm_area_struct *vma = vmf->vma;
struct vfio_pci_device *vdev = vma->vm_private_data;
vm_fault_t ret = VM_FAULT_NOPAGE;
unsigned long vaddr, pfn;
pte_t *ptep;
spinlock_t *ptl;

mutex_lock(>vma_lock);
down_read(>memory_lock);

if (!__vfio_pci_memory_enabled(vdev)) {
ret = VM_FAULT_SIGBUS;
goto up_out;
}

if (!follow_pte(vma->vm_mm, vma->vm_start, , ))
goto up_out;

for (vaddr = vma->start, pfn = vma->vm_pgoff; vaddr < vma->end;) {
ret = vmf_insert_pfn(vma, vaddr, pfn);
if (ret)
goto zap_vma;
vaddr += PAGE_SIZE;
pfn += 1;
}

if (__vfio_pci_add_vma(vdev, vma)) {
ret = VM_FAULT_OOM;
goto zap_vma;
}

mutex_unlock(>vma_lock);
up_read(>memory_lock);
return ret;

zap_vma:
zap_vma_ptes(vma, vma->vm_start, vaddr - vma->vm_start);
up_out:
mutex_unlock(>vma_lock);
up_read(>memory_lock);
return ret;
}

> -邮件原件-
> 发件人: Peter Xu [mailto:pet...@redhat.com]
> 发送时间: 2021年3月9日 6:56
> 收件人: Alex Williamson 
> 抄送: Zeng Tao ; linux...@huawei.com; Cornelia
> Huck ; Kevin Tian ; Andrew
> Morton ; Giovanni Cabiddu
> ; Michel Lespinasse ; Jann
> Horn ; Max Gurtovoy ;
> k...@vger.kernel.org; linux-kernel@vger.kernel.org; Jason Gunthorpe
> 
> 主题: Re: [PATCH] vfio/pci: make the vfio_pci_mmap_fault reentrant
> 
> On Mon, Mar 08, 2021 at 01:21:06PM -0700, Alex Williamson wrote:
> > On Mon, 8 Mar 2021 19:11:26 +0800
> > Zeng Tao  wrote:
> >
> > > We have met the following error when test with DPDK testpmd:
> > > [ 1591.733256] kernel BUG at mm/memory.c:2177!
> > > [ 1591.739515] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP [
> > > 1591.747381] Modules linked in: vfio_iommu_type1 vfio_pci
> > > vfio_virqfd vfio pv680_mii(O) [ 1591.760536] CPU: 2 PID: 227 Comm:
> > > lcore-worker-2 Tainted: G O 5.11.0-rc3+ #1 [ 1591.770735] Hardware
> > > name:  , BIOS HiFPGA 1P B600 V121-1 [ 1591.778872] pstate:
> > > 4049 (nZcv daif +PAN -UAO -TCO BTYPE=--) [ 1591.786134] pc :
> > > remap_pfn_range+0x214/0x340 [ 1591.793564] lr :
> > > remap_pfn_range+0x1b8/0x340 [ 1591.799117] sp : 80001068bbd0 [
> > > 1591.803476] x29: 80001068bbd0 x28: 042eff6f [
> > > 1591.810404] x27: 00110091 x26: 00130091 [
> > > 1591.817457] x25: 00680fd3 x24: a92f1338e358 [
> > > 1591.825144] x23: 00114000 x22: 0041 [
> > > 1591.832506] x21: 00130091 x20: a92f141a4000 [
> > > 1591.839520] x19: 001100a0 x18:  [
> > > 1591.846108] x17:  x16: a92f11844540 [
> > > 1591.853570] x15:  x14:  [
> > > 1591.860768] x13: fc00 x12: 0880 [
> > > 1591.868053] x11: 0821bf3d01d0 x10: 5ef2abd89000 [
> > > 1591.875932] x9 : a92f12ab0064 x8 : a92f136471c0 [
> > > 1591.883208] x7 : 00114091 x6 : 0002 [
> > > 1591.890177] x5 : 0001 x4 : 0001 [
> > > 1591.896656] x3 :  x2 : 016804400fd3 [
> > > 1591.903215] x1 : 082126261880 x0 : fc2084989868 [
> > > 1591.910234] Call trace:
> > > [ 1591.914837]  remap_pfn_range+0x214/0x340 [ 1591.921765]
> > > vfio_pci_mmap_fault+0xac/0x130 [vfio_pci] [ 1591.931200]
> > > __do_fault+0x44/0x12c [ 1591.937031]  handle_mm_fault+0xcc8/0x1230 [
> > > 1591.942475]  do_page_fault+0x16c/0x484 [ 1591.948635]
> > > do_translation_fault+0xbc/0xd8 [ 1591.954171]
> > > do_mem_abort+0x4c/0xc0 [ 1591.960316]  el0_da+0x40/0x80 [
> > > 1591.965585]  el0_sync_handler+0x168/0x1b0 [ 1591.971608]
> > > el0_sync+0x174/0x180 [ 1591.978312] Code: eb1b027f 54c0 f9400022
> > > b4fffe02 (d421)
> > >
> > > The cause is that the vfio_pci_mmap_fault function is not reentrant,
> > > if multiple threads access the same address which will lead to a
> > > page fault at the same time, we will have the above error.
> > >
> > > Fix the issue by making the vfio_pci_mmap_fault reentrant, and there
> > > is another issue that when the io_remap_pfn_range fails, we need to
> > > undo the __vfio_pci_add_vma, fix it by moving the __vfio_pci_add_vma
> > > down after the io_remap_pfn_range.
> > >
> > > Fixes: 11c4cd07ba11 ("vfio-pci: Fault mmaps to enable vma tracking")
> > > Signed-off-by: Zeng Tao 
> > > ---
> > >  drivers/vfio/pci/vfio_pci.c | 14 ++
> > 

RE: [PATCH 00/35] Enhance memory utilization with DMEMFS

[Zengtao (B)]

> -Original Message-
> From: yulei.ker...@gmail.com [mailto:yulei.ker...@gmail.com]
> Sent: Thursday, October 08, 2020 3:54 PM
> To: a...@linux-foundation.org; naoya.horigu...@nec.com;
> v...@zeniv.linux.org.uk; pbonz...@redhat.com
> Cc: linux-fsde...@vger.kernel.org; k...@vger.kernel.org;
> linux-kernel@vger.kernel.org; xiaoguangrong.e...@gmail.com;
> kernel...@gmail.com; lihaiwei.ker...@gmail.com; Yulei Zhang
> Subject: [PATCH 00/35] Enhance memory utilization with DMEMFS
> 
> From: Yulei Zhang 
> 
> In current system each physical memory page is assocaited with
> a page structure which is used to track the usage of this page.
> But due to the memory usage rapidly growing in cloud environment,
> we find the resource consuming for page structure storage becomes
> highly remarkable. So is it an expense that we could spare?
> 
> This patchset introduces an idea about how to save the extra
> memory through a new virtual filesystem -- dmemfs.
> 
> Dmemfs (Direct Memory filesystem) is device memory or reserved
> memory based filesystem. This kind of memory is special as it
> is not managed by kernel and most important it is without 'struct page'.
> Therefore we can leverage the extra memory from the host system
> to support more tenants in our cloud service.
> 
> We uses a kernel boot parameter 'dmem=' to reserve the system
> memory when the host system boots up, the details can be checked
> in /Documentation/admin-guide/kernel-parameters.txt.
> 
> Theoretically for each 4k physical page it can save 64 bytes if
> we drop the 'struct page', so for guest memory with 320G it can
> save about 5G physical memory totally.

Sounds interesting, but seems your patch only support x86, have you
 considered aarch64?

Regards
Zengtao 

RE: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns

[Zengtao (B)]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Tuesday, September 15, 2020 8:45 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; Vincenzo Frascino; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] time: Avoid undefined behaviour in
> timespec64_to_ns
> 
> On Tue, Sep 15, 2020 at 2:20 PM Zengtao (B)
>  wrote:
> 
> > > > Fixes: bd40a175769d ("y2038: itimer: change implementation to
> > > timespec64")
> > >
> > > This one caused the regression, but if we add the check here, it
> > > may be best to also add it in prior kernels that may have the same
> > > bug in other callers of the same function. Maybe backport all the
> > > way to stable kernels that first added timespec64?
> > >
> >
> > I think we need to do the backport, but not sure about the start
> point
> > Thanks for your review.
> 
> I would suggest
> 
> Cc:  # v3.17+
> Fixes: 361a3bf00582 ("time64: Add time64.h header and define
> struct timespec64")

Yes, make sense, commit 361a3bf00582 introduce a potential issue and
 commit bd40a175769d trigger the issue.

Regards
Zengtao 

> 
>Arnd

RE: [PATCH] time: Avoid undefined behaviour in timespec64_to_ns

[Zengtao (B)]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Tuesday, September 01, 2020 8:47 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; Vincenzo Frascino; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] time: Avoid undefined behaviour in
> timespec64_to_ns
> 
> On Tue, Sep 1, 2020 at 11:32 AM Zeng Tao 
> wrote:
> >
> > Since commit bd40a175769d ("y2038: itimer: change
> implementation to timespec64")
> > we have break the time clamping which handles the potential
> overflow.
> 
> Indeed, good catch!
> 
> And I broke it despite the comment telling me about the problem.
> 
> > In this patch, we fix it in the timespec64_to_ns because there is
> > possiblity to cause the same undefined behaviour on overflow
> whenever
> > the function is called.
> 
> I checked the most important callers of this function, and I agree
> that truncating at the maximum would be sensible in most cases
> here.
> 
> In timekeeping_init(), there is already a check for
> timespec64_valid_settod() that limits it even further, but that
> wouldn't make sense for most callers.
> 
> > Fixes: bd40a175769d ("y2038: itimer: change implementation to
> timespec64")
> 
> This one caused the regression, but if we add the check here, it
> may be best to also add it in prior kernels that may have the same
> bug in other callers of the same function. Maybe backport all the
> way to stable kernels that first added timespec64?
> 

I think we need to do the backport, but not sure about the start point
Thanks for your review. 

> Cc  # v3.17+
> 
> > Signed-off-by: Zeng Tao 
> 
> Reviewed-by: Arnd Bergmann 

RE: [PATCH] arm64: topology: Stop using MPIDR for topology information

[Zengtao (B)]

> -Original Message-
> From: Valentin Schneider [mailto:valentin.schnei...@arm.com]
> Sent: Wednesday, September 02, 2020 6:52 PM
> To: Zengtao (B)
> Cc: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
> Catalin Marinas; Will Deacon; Sudeep Holla; Robin Murphy; Jeremy Linton;
> Dietmar Eggemann; Morten Rasmussen
> Subject: Re: [PATCH] arm64: topology: Stop using MPIDR for topology
> information
> 
> 
> On 02/09/20 04:24, B wrote:
> > Hi Valentin:
> >
> >> -Original Message-
> >> From: Valentin Schneider [mailto:valentin.schnei...@arm.com]
> >> Sent: Saturday, August 29, 2020 9:00 PM
> >> To: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> >> Cc: Catalin Marinas; Will Deacon; Sudeep Holla; Robin Murphy; Jeremy
> >> Linton; Dietmar Eggemann; Morten Rasmussen; Zengtao (B)
> >> Subject: [PATCH] arm64: topology: Stop using MPIDR for topology
> >> information
> >>
> >> In the absence of ACPI or DT topology data, we fallback to haphazardly
> >> decoding *something* out of MPIDR. Sadly, the contents of that
> register
> >> are
> >> mostly unusable due to the implementation leniancy and things like Aff0
> >> having to be capped to 15 (despite being encoded on 8 bits).
> >>
> >> Consider a simple system with a single package of 32 cores, all under
> the
> >> same LLC. We ought to be shoving them in the same core_sibling mask,
> >> but
> >> MPIDR is going to look like:
> >>
> >>   | CPU  | 0 | ... | 15 | 16 | ... | 31 |
> >>   |--+---+-+++-++
> >>   | Aff0 | 0 | ... | 15 |  0 | ... | 15 |
> >>   | Aff1 | 0 | ... |  0 |  1 | ... |  1 |
> >>   | Aff2 | 0 | ... |  0 |  0 | ... |  0 |
> >>
> >> Which will eventually yield
> >>
> >>   core_sibling(0-15)  == 0-15
> >>   core_sibling(16-31) == 16-31
> >>
> >> NUMA woes
> >> =
> >>
> >> If we try to play games with this and set up NUMA boundaries within
> those
> >> groups of 16 cores via e.g. QEMU:
> >>
> >>   # Node0: 0-9; Node1: 10-19
> >>   $ qemu-system-aarch64  \
> >> -smp 20 -numa node,cpus=0-9,nodeid=0 -numa
> >> node,cpus=10-19,nodeid=1
> >>
> >> The scheduler's MC domain (all CPUs with same LLC) is going to be built
> via
> >>
> >>   arch_topology.c::cpu_coregroup_mask()
> >>
> >> In there we try to figure out a sensible mask out of the topology
> >> information we have. In short, here we'll pick the smallest of NUMA or
> >> core sibling mask.
> >>
> >>   node_mask(CPU9)== 0-9
> >>   core_sibling(CPU9) == 0-15
> >>
> >> MC mask for CPU9 will thus be 0-9, not a problem.
> >>
> >>   node_mask(CPU10)== 10-19
> >>   core_sibling(CPU10) == 0-15
> >>
> >> MC mask for CPU10 will thus be 10-19, not a problem.
> >>
> >>   node_mask(CPU16)== 10-19
> >>   core_sibling(CPU16) == 16-19
> >>
> >> MC mask for CPU16 will thus be 16-19... Uh oh. CPUs 16-19 are in two
> >> different unique MC spans, and the scheduler has no idea what to make
> of
> >> that. That triggers the WARN_ON() added by commit
> >>
> >>   ccf74128d66c ("sched/topology: Assert non-NUMA topology masks
> >> don't (partially) overlap")
> >>
> >> Fixing MPIDR-derived topology
> >> =
> >>
> >> We could try to come up with some cleverer scheme to figure out which
> of
> >> the available masks to pick, but really if one of those masks resulted
> from
> >> MPIDR then it should be discarded because it's bound to be bogus.
> >>
> >> I was hoping to give MPIDR a chance for SMT, to figure out which
> threads
> >> are
> >> in the same core using Aff1-3 as core ID, but Sudeep and Robin pointed
> out
> >> to me that there are systems out there where *all* cores have
> non-zero
> >> values in their higher affinity fields (e.g. RK3288 has "5" in all of its
> >> cores' MPIDR.Aff1), which would expose a bogus core ID to userspace.
> >>
> >> Stop using MPIDR for topology information. When no other source of
> >> topology
> >> information is available, mark each CPU as its own core and its NUMA
> >> node
> >> as its LLC domain.
> >
> > I agree with your idea to remove the topology functionality of MPIDR ,
> > but I think we need also consider ARM32 and GIC.
> >
> 
> Could you please elaborate? This change doesn't impact arch_topology, so
> only arm64 is affected.

Yes, this change only affects arm64, my question is that do we need to
 leverage it to arm32 since arm32 got the same issue.

And for GIC we are also using MPIDR for the topology info, but I am sure
It's got the same issue or not, just a suggestion to have a look.

RE: [PATCH] arm64: topology: Stop using MPIDR for topology information

[Zengtao (B)]

Hi Valentin:

> -Original Message-
> From: Valentin Schneider [mailto:valentin.schnei...@arm.com]
> Sent: Saturday, August 29, 2020 9:00 PM
> To: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org
> Cc: Catalin Marinas; Will Deacon; Sudeep Holla; Robin Murphy; Jeremy
> Linton; Dietmar Eggemann; Morten Rasmussen; Zengtao (B)
> Subject: [PATCH] arm64: topology: Stop using MPIDR for topology
> information
> 
> In the absence of ACPI or DT topology data, we fallback to haphazardly
> decoding *something* out of MPIDR. Sadly, the contents of that register
> are
> mostly unusable due to the implementation leniancy and things like Aff0
> having to be capped to 15 (despite being encoded on 8 bits).
> 
> Consider a simple system with a single package of 32 cores, all under the
> same LLC. We ought to be shoving them in the same core_sibling mask,
> but
> MPIDR is going to look like:
> 
>   | CPU  | 0 | ... | 15 | 16 | ... | 31 |
>   |--+---+-+++-++
>   | Aff0 | 0 | ... | 15 |  0 | ... | 15 |
>   | Aff1 | 0 | ... |  0 |  1 | ... |  1 |
>   | Aff2 | 0 | ... |  0 |  0 | ... |  0 |
> 
> Which will eventually yield
> 
>   core_sibling(0-15)  == 0-15
>   core_sibling(16-31) == 16-31
> 
> NUMA woes
> =
> 
> If we try to play games with this and set up NUMA boundaries within those
> groups of 16 cores via e.g. QEMU:
> 
>   # Node0: 0-9; Node1: 10-19
>   $ qemu-system-aarch64  \
> -smp 20 -numa node,cpus=0-9,nodeid=0 -numa
> node,cpus=10-19,nodeid=1
> 
> The scheduler's MC domain (all CPUs with same LLC) is going to be built via
> 
>   arch_topology.c::cpu_coregroup_mask()
> 
> In there we try to figure out a sensible mask out of the topology
> information we have. In short, here we'll pick the smallest of NUMA or
> core sibling mask.
> 
>   node_mask(CPU9)== 0-9
>   core_sibling(CPU9) == 0-15
> 
> MC mask for CPU9 will thus be 0-9, not a problem.
> 
>   node_mask(CPU10)== 10-19
>   core_sibling(CPU10) == 0-15
> 
> MC mask for CPU10 will thus be 10-19, not a problem.
> 
>   node_mask(CPU16)== 10-19
>   core_sibling(CPU16) == 16-19
> 
> MC mask for CPU16 will thus be 16-19... Uh oh. CPUs 16-19 are in two
> different unique MC spans, and the scheduler has no idea what to make of
> that. That triggers the WARN_ON() added by commit
> 
>   ccf74128d66c ("sched/topology: Assert non-NUMA topology masks
> don't (partially) overlap")
> 
> Fixing MPIDR-derived topology
> =
> 
> We could try to come up with some cleverer scheme to figure out which of
> the available masks to pick, but really if one of those masks resulted from
> MPIDR then it should be discarded because it's bound to be bogus.
> 
> I was hoping to give MPIDR a chance for SMT, to figure out which threads
> are
> in the same core using Aff1-3 as core ID, but Sudeep and Robin pointed out
> to me that there are systems out there where *all* cores have non-zero
> values in their higher affinity fields (e.g. RK3288 has "5" in all of its
> cores' MPIDR.Aff1), which would expose a bogus core ID to userspace.
> 
> Stop using MPIDR for topology information. When no other source of
> topology
> information is available, mark each CPU as its own core and its NUMA
> node
> as its LLC domain.

I agree with your idea to remove the topology functionality of MPIDR ,
but I think we need also consider ARM32 and GIC. 

> 
> Signed-off-by: Valentin Schneider 
> ---
>  arch/arm64/kernel/topology.c | 32 +---
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/kernel/topology.c b/arch/arm64/kernel/topology.c
> index 0801a0f3c156..ff1dd1dbfe64 100644
> --- a/arch/arm64/kernel/topology.c
> +++ b/arch/arm64/kernel/topology.c
> @@ -36,21 +36,23 @@ void store_cpu_topology(unsigned int cpuid)
>   if (mpidr & MPIDR_UP_BITMASK)
>   return;
> 
> - /* Create cpu topology mapping based on MPIDR. */
> - if (mpidr & MPIDR_MT_BITMASK) {
> - /* Multiprocessor system : Multi-threads per core */
> - cpuid_topo->thread_id  = MPIDR_AFFINITY_LEVEL(mpidr, 0);
> - cpuid_topo->core_id= MPIDR_AFFINITY_LEVEL(mpidr, 1);
> - cpuid_topo->package_id = MPIDR_AFFINITY_LEVEL(mpidr, 2) |
> -  MPIDR_AFFINITY_LEVEL(mpidr, 3) << 8;
> - } else {
> - /* Multiprocessor system : Single-thread per core */
> - cpuid_topo->thread_id  = -1;
> - cpuid_topo->core_id= MPIDR_AFFINITY_LEVEL(mpidr, 0);
> - cpuid_topo->package

RE: [PATCH] vfio/pci: fix racy on error and request eventfd ctx

[Zengtao (B)]

> -Original Message-
> From: Qian Cai [mailto:c...@lca.pw]
> Sent: Wednesday, July 15, 2020 9:34 PM
> To: Zengtao (B)
> Cc: alex.william...@redhat.com; Cornelia Huck; Kevin Tian; Peter Xu;
> Andrew Morton; Michel Lespinasse; Denis Efremov; k...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] vfio/pci: fix racy on error and request eventfd ctx
> 
> On Wed, Jul 15, 2020 at 03:34:41PM +0800, Zeng Tao wrote:
> > The vfio_pci_release call will free and clear the error and request
> > eventfd ctx while these ctx could be in use at the same time in the
> > function like vfio_pci_request, and it's expected to protect them under
> > the vdev->igate mutex, which is missing in vfio_pci_release.
> 
> How about other similar places calling eventfd_ctx_put() for "struct
> vfio_pci_device" ? For example, vfio_intx_set_signal().
>
I think there is no need, since the only wrapper call is
vfio_pci_set_irqs_ioctl which is already protected by the igate mutex.
 
> >
> > This issue is introduced since commit 1518ac272e78 ("vfio/pci: fix
> memory
> > leaks of eventfd ctx"),and since commit 5c5866c593bb ("vfio/pci: Clear
> > error and request eventfd ctx after releasing"), it's very easily to
> > trigger the kernel panic like this:
> >
> > [ 9513.904346] Unable to handle kernel NULL pointer dereference at
> virtual address 0008
> > [ 9513.913091] Mem abort info:
> > [ 9513.915871]   ESR = 0x9606
> > [ 9513.918912]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 9513.924198]   SET = 0, FnV = 0
> > [ 9513.927238]   EA = 0, S1PTW = 0
> > [ 9513.930364] Data abort info:
> > [ 9513.933231]   ISV = 0, ISS = 0x0006
> > [ 9513.937048]   CM = 0, WnR = 0
> > [ 9513.940003] user pgtable: 4k pages, 48-bit VAs,
> pgdp=007ec7d12000
> > [ 9513.946414] [0008] pgd=007ec7d13003,
> p4d=007ec7d13003, pud=007ec728c003,
> pmd=
> > [ 9513.956975] Internal error: Oops: 9606 [#1] PREEMPT SMP
> > [ 9513.962521] Modules linked in: vfio_pci vfio_virqfd vfio_iommu_type1
> vfio hclge hns3 hnae3 [last unloaded: vfio_pci]
> > [ 9513.972998] CPU: 4 PID: 1327 Comm: bash Tainted: GW
> 5.8.0-rc4+ #3
> > [ 9513.980443] Hardware name: Huawei TaiShan 2280 V2/BC82AMDC,
> BIOS 2280-V2 CS V3.B270.01 05/08/2020
> > [ 9513.989274] pstate: 80400089 (Nzcv daIf +PAN -UAO BTYPE=--)
> > [ 9513.994827] pc : _raw_spin_lock_irqsave+0x48/0x88
> > [ 9513.999515] lr : eventfd_signal+0x6c/0x1b0
> > [ 9514.003591] sp : 800038a0b960
> > [ 9514.006889] x29: 800038a0b960 x28: 007ef7f4da10
> > [ 9514.012175] x27: 207eefbbfc80 x26: bb7903457000
> > [ 9514.017462] x25: bb7912191000 x24: 007ef7f4d400
> > [ 9514.022747] x23: 20be6e0e4c00 x22: 0008
> > [ 9514.028033] x21:  x20: 
> > [ 9514.033321] x19: 0008 x18: 
> > [ 9514.038606] x17:  x16: bb7910029328
> > [ 9514.043893] x15:  x14: 0001
> > [ 9514.049179] x13:  x12: 0002
> > [ 9514.054466] x11:  x10: 0a00
> > [ 9514.059752] x9 : 800038a0b840 x8 : 007ef7f4de60
> > [ 9514.065038] x7 : 007fffc96690 x6 : fe01faffb748
> > [ 9514.070324] x5 :  x4 : 
> > [ 9514.075609] x3 :  x2 : 0001
> > [ 9514.080895] x1 : 007ef7f4d400 x0 : 
> > [ 9514.086181] Call trace:
> > [ 9514.088618]  _raw_spin_lock_irqsave+0x48/0x88
> > [ 9514.092954]  eventfd_signal+0x6c/0x1b0
> > [ 9514.096691]  vfio_pci_request+0x84/0xd0 [vfio_pci]
> > [ 9514.101464]  vfio_del_group_dev+0x150/0x290 [vfio]
> > [ 9514.106234]  vfio_pci_remove+0x30/0x128 [vfio_pci]
> > [ 9514.111007]  pci_device_remove+0x48/0x108
> > [ 9514.115001]  device_release_driver_internal+0x100/0x1b8
> > [ 9514.120200]  device_release_driver+0x28/0x38
> > [ 9514.124452]  pci_stop_bus_device+0x68/0xa8
> > [ 9514.128528]  pci_stop_and_remove_bus_device+0x20/0x38
> > [ 9514.133557]  pci_iov_remove_virtfn+0xb4/0x128
> > [ 9514.137893]  sriov_disable+0x3c/0x108
> > [ 9514.141538]  pci_disable_sriov+0x28/0x38
> > [ 9514.145445]  hns3_pci_sriov_configure+0x48/0xb8 [hns3]
> > [ 9514.150558]  sriov_numvfs_store+0x110/0x198
> > [ 9514.154724]  dev_attr_store+0x44/0x60
> > [ 9514.158373]  sysfs_kf_write+0x5c/0x78
> > [ 9514.162018]  kernfs_fop_write+0x104/0x210
> > [ 9514.166010]  __vfs_write+0x48/0x90
> > [ 9514.169395]  vfs_write+0xb

RE: [PATCH] vfio/pci: fix racy on error and request eventfd ctx

[Zengtao (B)]

> -Original Message-
> From: kvm-ow...@vger.kernel.org [mailto:kvm-ow...@vger.kernel.org]
> On Behalf Of Cornelia Huck
> Sent: Wednesday, July 15, 2020 6:40 PM
> To: Zengtao (B)
> Cc: alex.william...@redhat.com; c...@lca.pw; Kevin Tian; Peter Xu;
> Andrew Morton; Michel Lespinasse; Denis Efremov; k...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] vfio/pci: fix racy on error and request eventfd ctx
> 
> On Wed, 15 Jul 2020 15:34:41 +0800
> Zeng Tao  wrote:
> 
> > The vfio_pci_release call will free and clear the error and request
> > eventfd ctx while these ctx could be in use at the same time in the
> > function like vfio_pci_request, and it's expected to protect them under
> > the vdev->igate mutex, which is missing in vfio_pci_release.
> >
> > This issue is introduced since commit 1518ac272e78 ("vfio/pci: fix
> memory
> > leaks of eventfd ctx"),and since commit 5c5866c593bb ("vfio/pci: Clear
> > error and request eventfd ctx after releasing"), it's very easily to
> > trigger the kernel panic like this:
> >
> > [ 9513.904346] Unable to handle kernel NULL pointer dereference at
> virtual address 0008
> > [ 9513.913091] Mem abort info:
> > [ 9513.915871]   ESR = 0x9606
> > [ 9513.918912]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 9513.924198]   SET = 0, FnV = 0
> > [ 9513.927238]   EA = 0, S1PTW = 0
> > [ 9513.930364] Data abort info:
> > [ 9513.933231]   ISV = 0, ISS = 0x0006
> > [ 9513.937048]   CM = 0, WnR = 0
> > [ 9513.940003] user pgtable: 4k pages, 48-bit VAs,
> pgdp=007ec7d12000
> > [ 9513.946414] [0008] pgd=007ec7d13003,
> p4d=007ec7d13003, pud=007ec728c003,
> pmd=
> > [ 9513.956975] Internal error: Oops: 9606 [#1] PREEMPT SMP
> > [ 9513.962521] Modules linked in: vfio_pci vfio_virqfd vfio_iommu_type1
> vfio hclge hns3 hnae3 [last unloaded: vfio_pci]
> > [ 9513.972998] CPU: 4 PID: 1327 Comm: bash Tainted: GW
> 5.8.0-rc4+ #3
> > [ 9513.980443] Hardware name: Huawei TaiShan 2280 V2/BC82AMDC,
> BIOS 2280-V2 CS V3.B270.01 05/08/2020
> > [ 9513.989274] pstate: 80400089 (Nzcv daIf +PAN -UAO BTYPE=--)
> > [ 9513.994827] pc : _raw_spin_lock_irqsave+0x48/0x88
> > [ 9513.999515] lr : eventfd_signal+0x6c/0x1b0
> > [ 9514.003591] sp : 800038a0b960
> > [ 9514.006889] x29: 800038a0b960 x28: 007ef7f4da10
> > [ 9514.012175] x27: 207eefbbfc80 x26: bb7903457000
> > [ 9514.017462] x25: bb7912191000 x24: 007ef7f4d400
> > [ 9514.022747] x23: 20be6e0e4c00 x22: 0008
> > [ 9514.028033] x21:  x20: 
> > [ 9514.033321] x19: 0008 x18: 
> > [ 9514.038606] x17:  x16: bb7910029328
> > [ 9514.043893] x15:  x14: 0001
> > [ 9514.049179] x13:  x12: 0002
> > [ 9514.054466] x11:  x10: 0a00
> > [ 9514.059752] x9 : 800038a0b840 x8 : 007ef7f4de60
> > [ 9514.065038] x7 : 007fffc96690 x6 : fe01faffb748
> > [ 9514.070324] x5 :  x4 : 
> > [ 9514.075609] x3 :  x2 : 0001
> > [ 9514.080895] x1 : 007ef7f4d400 x0 : 
> > [ 9514.086181] Call trace:
> > [ 9514.088618]  _raw_spin_lock_irqsave+0x48/0x88
> > [ 9514.092954]  eventfd_signal+0x6c/0x1b0
> > [ 9514.096691]  vfio_pci_request+0x84/0xd0 [vfio_pci]
> > [ 9514.101464]  vfio_del_group_dev+0x150/0x290 [vfio]
> > [ 9514.106234]  vfio_pci_remove+0x30/0x128 [vfio_pci]
> > [ 9514.111007]  pci_device_remove+0x48/0x108
> > [ 9514.115001]  device_release_driver_internal+0x100/0x1b8
> > [ 9514.120200]  device_release_driver+0x28/0x38
> > [ 9514.124452]  pci_stop_bus_device+0x68/0xa8
> > [ 9514.128528]  pci_stop_and_remove_bus_device+0x20/0x38
> > [ 9514.133557]  pci_iov_remove_virtfn+0xb4/0x128
> > [ 9514.137893]  sriov_disable+0x3c/0x108
> > [ 9514.141538]  pci_disable_sriov+0x28/0x38
> > [ 9514.145445]  hns3_pci_sriov_configure+0x48/0xb8 [hns3]
> > [ 9514.150558]  sriov_numvfs_store+0x110/0x198
> > [ 9514.154724]  dev_attr_store+0x44/0x60
> > [ 9514.158373]  sysfs_kf_write+0x5c/0x78
> > [ 9514.162018]  kernfs_fop_write+0x104/0x210
> > [ 9514.166010]  __vfs_write+0x48/0x90
> > [ 9514.169395]  vfs_write+0xbc/0x1c0
> > [ 9514.172694]  ksys_write+0x74/0x100
> > [ 9514.176079]  __arm64_sys_write+0x24/0x30
> > [ 9514.179987]  el0_svc_common.constprop.4+0x110/0x200
> > [ 9514.184842]  do_el0_svc+0x

RE: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Zengtao (B)]

Hi maxime: 

>-Original Message-
>From: Maxime Ripard [mailto:maxime.rip...@bootlin.com]
>Sent: Thursday, July 18, 2019 12:38 AM
>To: Zengtao (B) 
>Cc: kis...@ti.com; Chen-Yu Tsai ; Paul Kocialkowski
>; Sakari Ailus ;
>linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org
>Subject: Re: [PATCH] phy: Change the configuration interface param to void*
>to make it more general
>
>Hi,
>
>On Wed, Jul 17, 2019 at 06:36:09AM +, Zengtao (B) wrote:
>> Hi Maxime:
>>
>> Thanks for your reply.
>>
>> >-Original Message-
>> >From: Maxime Ripard [mailto:maxime.rip...@bootlin.com]
>> >Sent: Thursday, July 11, 2019 7:21 PM
>> >To: Zengtao (B) 
>> >Cc: kis...@ti.com; Chen-Yu Tsai ; Paul Kocialkowski
>> >; Sakari Ailus
>> >; linux-kernel@vger.kernel.org;
>> >linux-arm-ker...@lists.infradead.org
>> >Subject: Re: [PATCH] phy: Change the configuration interface param to
>> >void* to make it more general
>> >
>> >* PGP Signed by an unknown key
>> >
>> >On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
>> >> The phy framework now allows runtime configurations, but only
>> >> limited to mipi now, and it's not reasonable to introduce user
>> >> specified configurations into the union phy_configure_opts
>> >> structure. An simple way is to replace with a void *.
>> >
>> >I'm not sure why it's unreasonable?
>> >
>>
>> The phy.h will need to include vendor specific phy headers
>
>I'm not sure this is an issue.
>
>> and the union phy_configure_opts will become more complex.
>
>And this was the plan all along.
>
>> I don't think this is a good solution to include all vendor specific
>> phy configs into a single union structure.
>
>The thing is, as Sakari have stated, this interface was meant as a generic way
>to negotiate a configuration between a PHY consumer and a PHY provider (ie,
>whatever sends data to the phy and the phy itself).
>
>If you remove the explicit type check, then you need to have prior knowledge
>(and agreement) on both sides, which breaks the initial intent.

I get your point, so if we follow your design, it will look this:

union phy_configure_opts {
struct 1_phy phy1_opts;
struct 1_phy phy2_opts;
struct 1_phy phy3_opts;
.
}

And the general phy framework needn't to know about the type but just pass 
through, 
the caller and the phy driver definitely need to know what they are doing.
So I suggest a more general type void *, otherwise the general phy will need to 
see a lot 
of details which is not that general. 

Zengtao 

>
>Maxime
>
>--
>Maxime Ripard, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com

RE: [PATCH] phy: Change the configuration interface param to void* to make it more general

[Zengtao (B)]

Hi Maxime:

Thanks for your reply.

>-Original Message-
>From: Maxime Ripard [mailto:maxime.rip...@bootlin.com]
>Sent: Thursday, July 11, 2019 7:21 PM
>To: Zengtao (B) 
>Cc: kis...@ti.com; Chen-Yu Tsai ; Paul Kocialkowski
>; Sakari Ailus ;
>linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org
>Subject: Re: [PATCH] phy: Change the configuration interface param to void*
>to make it more general
>
>* PGP Signed by an unknown key
>
>On Fri, Jul 12, 2019 at 02:04:08AM +0800, Zeng Tao wrote:
>> The phy framework now allows runtime configurations, but only limited
>> to mipi now, and it's not reasonable to introduce user specified
>> configurations into the union phy_configure_opts structure. An simple
>> way is to replace with a void *.
>
>I'm not sure why it's unreasonable?
>
The phy.h will need to include vendor specific phy headers, and the union 
phy_configure_opts
will become more complex. I don't think this is a good solution to include all 
vendor specific phy
configs into a single union structure. 

>> We have already got some phy drivers which introduce private phy API
>> for runtime configurations, and with this patch, they can switch to
>> the phy_configure as a replace.
>
>If you have a custom mode of operation, then you'll need a custom
>phy_mode as well, and surely you can have a custom set of parameters.
>
>Since those functions are meant to provide a two-way negotiation of the
>various parameters, you'll have to have that structure shared between the
>two either way, so the only thing required in addition to what you would have
>passing a void is one line to add that structure in the union.
>
>That's barely unreasonable.
>
>Maxime
>
>--
>Maxime Ripard, Bootlin
>Embedded Linux and Kernel engineering
>https://bootlin.com
>
>* Unknown Key
>* 0x671851C5

RE: [PATCH] usb: dwc3: gadget: issue a stop command for ISOC endpoint

[Zengtao (B)]

Hi balbi:

I got this issue in the UVC application, and I think this issue still exist in 
the 
latest dwc3 driver.

And we should issue an stop command and the queue is empty when it's 
ISOC transfer, otherwise we will end up with MISS ISOC error for all the 
upcoming transfers.

So I think you can test the UVC application with the latest driver.

Regards
Zengtao 

>-Original Message-
>From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
>Sent: Monday, January 21, 2019 4:56 PM
>To: Zengtao (B) 
>Cc: Greg Kroah-Hartman ;
>linux-...@vger.kernel.org; linux-kernel@vger.kernel.org; Laurent
>Pinchart 
>Subject: RE: [PATCH] usb: dwc3: gadget: issue a stop command for ISOC
>endpoint
>
>* PGP Signed by an unknown key
>
>
>Hi,
>
>"Zengtao (B)"  writes:
>>>-Original Message-
>>>From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
>>>Sent: Monday, January 21, 2019 4:17 PM
>>>To: Zengtao (B) 
>>>Cc: Zengtao (B) ; Greg Kroah-Hartman
>>>; linux-...@vger.kernel.org;
>>>linux-kernel@vger.kernel.org
>>>Subject: Re: [PATCH] usb: dwc3: gadget: issue a stop command for
>ISOC
>>>endpoint
>>>
>>>> Old Signed by an unknown key
>>>
>>>
>>>Hi,
>>>
>>>Zeng Tao  writes:
>>>> For ISOC transfers, if there is no available data for a period, we
>>>> need to stop the transfer by issue a stop command, otherwise, all
>>>> the upcoming transfers will started by update transfer command,
>and
>>>> will be dropped with MISS ISOC errors.
>>>
>>>We, actually, have code to handle missed isoc errors now. Have you
>>>tested with that applied? Which kernel are you using? Can you share
>>>tracepoints captured with v5.0-rc3?
>>>
>>
>> Not v5.0-rc3, but I tested it based on 4.9 with backported dwc3
>> driver. And with UVC application, it's very easy to reproduce it.
>
>Ok, so it could be that you missed patches while backporting, right?
>How can I reproduce this? What do I need? Care to give detailed
>instructions?
>
>--
>balbi
>
>* Unknown Key
>* 0xE11A9906

RE: [PATCH] usb: dwc3: gadget: don't remove the request if bus-expired

[Zengtao (B)]

Hi balbi:

>-Original Message-
>From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
>Sent: Monday, January 14, 2019 4:13 PM
>To: Zengtao (B) 
>Cc: Zengtao (B) ; Greg Kroah-Hartman
>; linux-...@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] usb: dwc3: gadget: don't remove the request if
>bus-expired
>
>* PGP Signed by an unknown key
>
>
>Hi,
>
>Zeng Tao  writes:
>> We have already returned EAGAIN for bus-expiry, and it's designed to
>> start with a future Frame number and start the transfer again. So we
>> should not remove the request for that case.
>>
>> Signed-off-by: Zeng Tao 
>
>Do we need a Fixes tag here? How about Cc stable? Can you share
>tracepoints exposing the problem?
>

I am not sure that we need to Fixes tag, it's not related to any single patch, 
but
there is definitely something wrong, after rethinking it again, I found that 
there
are still some problems for this patch, for the reties inside the driver, we 
should not
remove the request, but if we return -EAGAIN to the gadget layer, we should 
because
the gadget will requeue the request again if we return -EAGAIN.

Any suggestions. 

>
>thanks
>
>--
>balbi
>
>* Unknown Key
>* 0xE11A9906

RE: [PATCH] usb: dwc3: gadget: issue a stop command for ISOC endpoint

[Zengtao (B)]

Hi balbi:

>-Original Message-
>From: Felipe Balbi [mailto:felipe.ba...@linux.intel.com]
>Sent: Monday, January 21, 2019 4:17 PM
>To: Zengtao (B) 
>Cc: Zengtao (B) ; Greg Kroah-Hartman
>; linux-...@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] usb: dwc3: gadget: issue a stop command for ISOC
>endpoint
>
>* PGP Signed by an unknown key
>
>
>Hi,
>
>Zeng Tao  writes:
>> For ISOC transfers, if there is no available data for a period, we
>> need to stop the transfer by issue a stop command, otherwise, all the
>> upcoming transfers will started by update transfer command, and will
>> be dropped with MISS ISOC errors.
>
>We, actually, have code to handle missed isoc errors now. Have you tested
>with that applied? Which kernel are you using? Can you share tracepoints
>captured with v5.0-rc3?
>

Not v5.0-rc3, but I tested it based on 4.9 with backported dwc3 driver. And with
UVC application, it's very easy to reproduce it.

>--
>balbi
>
>* Unknown Key
>* 0xE11A9906

RE: [PATCH] staging: android: ion: add buffer flag update ioctl

[Zengtao (B)]

>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Friday, January 04, 2019 9:53 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 1/3/19 5:42 PM, Zengtao (B) wrote:
>>> -Original Message-
>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>> Sent: Thursday, January 03, 2019 6:31 AM
>>> To: Zengtao (B) ;
>sumit.sem...@linaro.org
>>> Cc: Greg Kroah-Hartman ; Arve
>Hjønnevåg
>>> ; Todd Kjos ; Martijn
>Coenen
>>> ; Joel Fernandes ;
>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>> linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/23/18 6:47 PM, Zengtao (B) wrote:
>>>> Hi laura:
>>>>
>>>>> -Original Message-
>>>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>>>> Sent: Friday, December 21, 2018 4:50 AM
>>>>> To: Zengtao (B) ;
>>> sumit.sem...@linaro.org
>>>>> Cc: Greg Kroah-Hartman ; Arve
>>> Hjønnevåg
>>>>> ; Todd Kjos ; Martijn
>>> Coenen
>>>>> ; Joel Fernandes ;
>>>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>>>> linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>>> ioctl
>>>>>
>>>>> On 12/19/18 5:39 PM, Zengtao (B) wrote:
>>>>>> Hi laura:
>>>>>>
>>>>>>> -Original Message-
>>>>>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>>>>>> Sent: Thursday, December 20, 2018 2:10 AM
>>>>>>> To: Zengtao (B) ;
>>>>> sumit.sem...@linaro.org
>>>>>>> Cc: Greg Kroah-Hartman ; Arve
>>>>> Hjønnevåg
>>>>>>> ; Todd Kjos ; Martijn
>>>>> Coenen
>>>>>>> ; Joel Fernandes ;
>>>>>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>>>>>> linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>>>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag
>>>>>>> update ioctl
>>>>>>>
>>>>>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>>>>>> In some usecases, the buffer cached attribute is not determined
>>>>>>>> at allocation time, it's determined just before the real cpu
>mapping.
>>>>>>>> And from the memory view of point, a buffer should not have
>the
>>>>>>> cached
>>>>>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>>>>>> introduced the new ioctl command to target the requirement.
>>>>>>>>
>>>>>>>
>>>>>>> This is racy and error prone. Can you explain more what problem
>>> you
>>>>>>> are trying to solve?
>>>>>>
>>>>>> My use case is like this:
>>>>>> 1.  There are two process A and B, A takes case of ion buffer
>>>>>> allocation,
>>>>> and
>>>>>> pass the buffer fd to B, then B maps and uses it.
>>>>>> 2.  Process B need to map the buffer with different cached
>>>>>> attribute for different use case, for example, if the buffer is
>>>>>> used for pure software algorithm, then we need to map it as
>>>>>> cached, otherwise non-cached, and B needs to deal with both
>cases.
>>>>>> And unfortunately the mmap syscall takes no cached flags and we
>>>>>> can't decide the cache attribute when we are doing the mmap, so I
>>>>>> introduce new the ioctl even though I think the solution is not as
>>> good.
>>>>>>
>>>>>>
>>>>>
>>>>> Thanks for the explanation, this was about the use case I expected.
>>>>> I'm pretty sure I had this exact problem once upon a time and we
>>>>> didn't come up with a sol

RE: [PATCH] staging: android: ion: add buffer flag update ioctl

[Zengtao (B)]

>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Thursday, January 03, 2019 6:31 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/23/18 6:47 PM, Zengtao (B) wrote:
>> Hi laura:
>>
>>> -Original Message-
>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>> Sent: Friday, December 21, 2018 4:50 AM
>>> To: Zengtao (B) ;
>sumit.sem...@linaro.org
>>> Cc: Greg Kroah-Hartman ; Arve
>Hjønnevåg
>>> ; Todd Kjos ; Martijn
>Coenen
>>> ; Joel Fernandes ;
>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>> linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/19/18 5:39 PM, Zengtao (B) wrote:
>>>> Hi laura:
>>>>
>>>>> -Original Message-
>>>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>>>> Sent: Thursday, December 20, 2018 2:10 AM
>>>>> To: Zengtao (B) ;
>>> sumit.sem...@linaro.org
>>>>> Cc: Greg Kroah-Hartman ; Arve
>>> Hjønnevåg
>>>>> ; Todd Kjos ; Martijn
>>> Coenen
>>>>> ; Joel Fernandes ;
>>>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>>>> linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>>>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>>>> ioctl
>>>>>
>>>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>>>> In some usecases, the buffer cached attribute is not determined at
>>>>>> allocation time, it's determined just before the real cpu mapping.
>>>>>> And from the memory view of point, a buffer should not have the
>>>>> cached
>>>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>>>> introduced the new ioctl command to target the requirement.
>>>>>>
>>>>>
>>>>> This is racy and error prone. Can you explain more what problem
>you
>>>>> are trying to solve?
>>>>
>>>> My use case is like this:
>>>> 1.  There are two process A and B, A takes case of ion buffer
>>>> allocation,
>>> and
>>>>pass the buffer fd to B, then B maps and uses it.
>>>> 2.  Process B need to map the buffer with different cached attribute
>>>> for different use case, for example, if the buffer is used for pure
>>>> software algorithm, then we need to map it as cached, otherwise
>>>> non-cached, and B needs to deal with both cases.
>>>> And unfortunately the mmap syscall takes no cached flags and we
>>>> can't decide the cache attribute when we are doing the mmap, so I
>>>> introduce new the ioctl even though I think the solution is not as
>good.
>>>>
>>>>
>>>
>>> Thanks for the explanation, this was about the use case I expected.
>>> I'm pretty sure I had this exact problem once upon a time and we
>>> didn't come up with a solution. I'd still like to get rid of uncached
>>> buffers in general and just use cached buffers (see
>>> http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/201
>>> 8-N
>>> ovember/128842.html)
>>> What's your usecase for uncached buffers?
>>
>> Some buffers are only used by HW, in this case, we tend to use
>uncached buffers.
>> But sometimes the SW need to read few buffer contents for debug
>> purpose and no synchronization is needed.
>>
>
>If this is primarily for debug, that's not really a compelling reason to
>support uncached buffers. It's incredibly difficult to do uncached buffers
>correctly I'd rather spend effort on making the cached use cases work
>better.
>
No, not only for debug, I meant if the buffers are uncached, the SW will only 
mmap
them for debug or even don't need to mmap them.
So for the two kinds of buffers:
1.  uncached: only the HW need to access it, the SW don't need to mmap it or 
just for debug.
2.  cached: both the HW and the SW need to access it.
The ION is designed as a generalized memory manager, I think we should cover as 
many 
requi

RE: [PATCH] staging: android: ion: add buffer flag update ioctl

[Zengtao (B)]

Hi laura:

>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Friday, December 21, 2018 4:50 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 5:39 PM, Zengtao (B) wrote:
>> Hi laura:
>>
>>> -Original Message-
>>> From: Laura Abbott [mailto:labb...@redhat.com]
>>> Sent: Thursday, December 20, 2018 2:10 AM
>>> To: Zengtao (B) ;
>sumit.sem...@linaro.org
>>> Cc: Greg Kroah-Hartman ; Arve
>Hjønnevåg
>>> ; Todd Kjos ; Martijn
>Coenen
>>> ; Joel Fernandes ;
>>> de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>>> linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>>> Subject: Re: [PATCH] staging: android: ion: add buffer flag update
>>> ioctl
>>>
>>> On 12/19/18 9:19 AM, Zeng Tao wrote:
>>>> In some usecases, the buffer cached attribute is not determined at
>>>> allocation time, it's determined just before the real cpu mapping.
>>>> And from the memory view of point, a buffer should not have the
>>> cached
>>>> attribute util is really mapped by the cpu. So in this patch, we
>>>> introduced the new ioctl command to target the requirement.
>>>>
>>>
>>> This is racy and error prone. Can you explain more what problem you
>>> are trying to solve?
>>
>> My use case is like this:
>> 1.  There are two process A and B, A takes case of ion buffer allocation,
>and
>>   pass the buffer fd to B, then B maps and uses it.
>> 2.  Process B need to map the buffer with different cached attribute
>> for different use case, for example, if the buffer is used for pure
>> software algorithm, then we need to map it as cached, otherwise
>> non-cached, and B needs to deal with both cases.
>> And unfortunately the mmap syscall takes no cached flags and we can't
>> decide the cache attribute when we are doing the mmap, so I introduce
>> new the ioctl even though I think the solution is not as good.
>>
>>
>
>Thanks for the explanation, this was about the use case I expected.
>I'm pretty sure I had this exact problem once upon a time and we didn't
>come up with a solution. I'd still like to get rid of uncached buffers in
>general and just use cached buffers (see
>http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-N
>ovember/128842.html)
>What's your usecase for uncached buffers?

Some buffers are only used by HW, in this case, we tend to use uncached buffers.
But sometimes the SW need to read few buffer contents for debug purpose and
no synchronization is needed.

>
>>>
>>>> Signed-off-by: Zeng Tao 
>>>> ---
>>>>drivers/staging/android/ion/ion-ioctl.c |  4 
>>>>drivers/staging/android/ion/ion.c   | 17
>+
>>>>drivers/staging/android/ion/ion.h   |  1 +
>>>>drivers/staging/android/uapi/ion.h  | 22
>>> ++
>>>>4 files changed, 44 insertions(+)
>>>>
>>>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>>>> b/drivers/staging/android/ion/ion-ioctl.c
>>>> index a8d3cc4..60bb702 100644
>>>> --- a/drivers/staging/android/ion/ion-ioctl.c
>>>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>>>> @@ -12,6 +12,7 @@
>>>>
>>>>union ion_ioctl_arg {
>>>>struct ion_allocation_data allocation;
>>>> +  struct ion_buffer_flag_data update;
>>>>struct ion_heap_query query;
>>>>};
>>>>
>>>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int
>>>> cmd, unsigned long arg)
>>>>
>>>>break;
>>>>}
>>>> +  case ION_IOC_BUFFER_UPDATE:
>>>> +  ret = ion_buffer_update(data.update.fd, data.update.flags);
>>>> +  break;
>>>>case ION_IOC_HEAP_QUERY:
>>>>ret = ion_query_heaps();
>>>>break;
>>>> diff --git a/drivers/staging/android/ion/ion.c
>>>> b/drivers/staging/android/ion/ion.c
>>>> index 9907332..f1404dc 100644
>>>> --- a/drivers/staging/android/ion/ion

RE: [PATCH] staging: android: ion: add buffer flag update ioctl

[Zengtao (B)]

Hi laura:

>-Original Message-
>From: Laura Abbott [mailto:labb...@redhat.com]
>Sent: Thursday, December 20, 2018 2:10 AM
>To: Zengtao (B) ; sumit.sem...@linaro.org
>Cc: Greg Kroah-Hartman ; Arve Hjønnevåg
>; Todd Kjos ; Martijn Coenen
>; Joel Fernandes ;
>de...@driverdev.osuosl.org; dri-de...@lists.freedesktop.org;
>linaro-mm-...@lists.linaro.org; linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] staging: android: ion: add buffer flag update ioctl
>
>On 12/19/18 9:19 AM, Zeng Tao wrote:
>> In some usecases, the buffer cached attribute is not determined at
>> allocation time, it's determined just before the real cpu mapping.
>> And from the memory view of point, a buffer should not have the
>cached
>> attribute util is really mapped by the cpu. So in this patch, we
>> introduced the new ioctl command to target the requirement.
>>
>
>This is racy and error prone. Can you explain more what problem you are
>trying to solve?

My use case is like this:
1.  There are two process A and B, A takes case of ion buffer allocation, and
 pass the buffer fd to B, then B maps and uses it.
2.  Process B need to map the buffer with different cached attribute for 
different use case, for example, if the buffer is used for pure software 
algorithm, 
then we need to map it as cached, otherwise non-cached, and B needs to deal
with both cases.
And unfortunately the mmap syscall takes no cached flags and we can't decide 
the cache attribute when we are doing the mmap, so I introduce new the ioctl 
even though I think the solution is not as good. 


>
>> Signed-off-by: Zeng Tao 
>> ---
>>   drivers/staging/android/ion/ion-ioctl.c |  4 
>>   drivers/staging/android/ion/ion.c   | 17 +
>>   drivers/staging/android/ion/ion.h   |  1 +
>>   drivers/staging/android/uapi/ion.h  | 22
>++
>>   4 files changed, 44 insertions(+)
>>
>> diff --git a/drivers/staging/android/ion/ion-ioctl.c
>> b/drivers/staging/android/ion/ion-ioctl.c
>> index a8d3cc4..60bb702 100644
>> --- a/drivers/staging/android/ion/ion-ioctl.c
>> +++ b/drivers/staging/android/ion/ion-ioctl.c
>> @@ -12,6 +12,7 @@
>>
>>   union ion_ioctl_arg {
>>  struct ion_allocation_data allocation;
>> +struct ion_buffer_flag_data update;
>>  struct ion_heap_query query;
>>   };
>>
>> @@ -83,6 +84,9 @@ long ion_ioctl(struct file *filp, unsigned int cmd,
>> unsigned long arg)
>>
>>  break;
>>  }
>> +case ION_IOC_BUFFER_UPDATE:
>> +ret = ion_buffer_update(data.update.fd, data.update.flags);
>> +break;
>>  case ION_IOC_HEAP_QUERY:
>>  ret = ion_query_heaps();
>>  break;
>> diff --git a/drivers/staging/android/ion/ion.c
>> b/drivers/staging/android/ion/ion.c
>> index 9907332..f1404dc 100644
>> --- a/drivers/staging/android/ion/ion.c
>> +++ b/drivers/staging/android/ion/ion.c
>> @@ -436,6 +436,23 @@ int ion_alloc(size_t len, unsigned int
>heap_id_mask, unsigned int flags)
>>  return fd;
>>   }
>>
>> +int ion_buffer_update(unsigned int fd, unsigned int flags) {
>> +struct dma_buf *dmabuf;
>> +struct ion_buffer *buffer;
>> +
>> +dmabuf = dma_buf_get(fd);
>> +
>> +if (!dmabuf)
>> +return -EINVAL;
>> +
>> +buffer = dmabuf->priv;
>> +buffer->flags = flags;
>> +dma_buf_put(dmabuf);
>> +
>> +return 0;
>> +}
>> +
>>   int ion_query_heaps(struct ion_heap_query *query)
>>   {
>>  struct ion_device *dev = internal_dev; diff --git
>> a/drivers/staging/android/ion/ion.h
>> b/drivers/staging/android/ion/ion.h
>> index c006fc1..99bf9ab 100644
>> --- a/drivers/staging/android/ion/ion.h
>> +++ b/drivers/staging/android/ion/ion.h
>> @@ -199,6 +199,7 @@ int ion_heap_pages_zero(struct page *page,
>size_t size, pgprot_t pgprot);
>>   int ion_alloc(size_t len,
>>unsigned int heap_id_mask,
>>unsigned int flags);
>> +int ion_buffer_update(unsigned int fd, unsigned int flags);
>>
>>   /**
>>* ion_heap_init_shrinker
>> diff --git a/drivers/staging/android/uapi/ion.h
>> b/drivers/staging/android/uapi/ion.h
>> index 5d70098..99753fc 100644
>> --- a/drivers/staging/android/uapi/ion.h
>> +++ b/drivers/staging/android/uapi/ion.h
>> @@ -74,6 +74,20 @@ struct ion_allocation_data {
>>  __u32 unused;
>>   };
>>
>> +/**
>> + * struct ion_buffer_flag_data - metadata passed from userspace for
>> +up

RE: [PATCH] usb: dwc3: gadget: fix miss isoc issue introduced by IRQ latency

[Zengtao (B)]

Hi Sergei:

>-Original Message-
>From: Sergei Shtylyov [mailto:sergei.shtyl...@cogentembedded.com]
>Sent: Friday, December 14, 2018 7:18 PM
>To: Zengtao (B) ; ba...@kernel.org
>Cc: liangshengjun ; Greg Kroah-Hartman
>; linux-...@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] usb: dwc3: gadget: fix miss isoc issue introduced by
>IRQ latency
>
>Hello!
>
>On 12/14/2018 07:32 PM, Zeng Tao wrote:
>
>> If it's a busy system, some times when we start an isoc transfer, the
>> framenumber get from the event buffer may be already elasped, in this
>
>   Frame number? Else my spell checker trips. :-)
>
>>  case, we will get all the packets dropped due to miss isoc. And we
>> turn
>
>   Remove the leading space, please.
>
>> into transfer nothing, to fix this issue, we need to fix the
>> framenumber to make sure that it's not out of date.
>>
>> Signed-off-by: Liang Shengjun 
>> Signed-off-by: Zeng Tao 
>[...]
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 9f92ee0..b63bd72 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1263,6 +1263,15 @@ static int __dwc3_gadget_get_frame(struct
>dwc3 *dwc)
>>  return DWC3_DSTS_SOFFN(reg);
>>  }
>>
>> +static bool __dwc3_gadget_target_frame_elapsed(struct dwc3_ep
>*dep) {
>> +u16 cframe =  __dwc3_gadget_get_frame(dep->dwc);
>> +u16 eframe = dep->frame_number &
>DWC3_EVENT_PRAM_SOFFN_MASK;
>> +
>> +return (((eframe - cframe) & DWC3_EVENT_PRAM_SOFFN_MASK)
>> +> DWC3_EVENT_PRAM_MAX_SOFFN / 2);
>
>   Please leave > on the previous line.
>   And you surely know that the outer parens are unnecessary? :-)
>

Thanks for your kindly corrections. :-), I will apply them in the patch if we 
need.

>[...]
>
>MBR, Sergei

RE: [PATCH] usb: dwc3: gadget: fix miss isoc issue introduced by IRQ latency

[Zengtao (B)]

Hi Thinh:

>-Original Message-
>From: Thinh Nguyen [mailto:thinh.ngu...@synopsys.com]
>Sent: Saturday, December 15, 2018 5:43 AM
>To: Felipe Balbi ; Zengtao (B)
>
>Cc: liangshengjun ; Greg Kroah-Hartman
>; linux-...@vger.kernel.org;
>linux-kernel@vger.kernel.org; Thinh Nguyen
>
>Subject: Re: [PATCH] usb: dwc3: gadget: fix miss isoc issue introduced by
>IRQ latency
>
>Hi Zengtao,
>
>On 12/14/2018 3:24 AM, Felipe Balbi wrote:
>> Hi,
>>
>> "Zengtao (B)"  writes:
>>>> -Original Message-
>>>> From: Felipe Balbi [mailto:ba...@kernel.org]
>>>> Sent: Friday, December 14, 2018 4:52 PM
>>>> To: Zengtao (B) 
>>>> Cc: liangshengjun ; Zengtao (B)
>>>> ; Greg Kroah-Hartman
>>>> ; linux-...@vger.kernel.org;
>>>> linux-kernel@vger.kernel.org
>>>> Subject: Re: [PATCH] usb: dwc3: gadget: fix miss isoc issue
>>>> introduced by IRQ latency
>>>>
>>>> * PGP Signed by an unknown key
>>>>
>>>> Zeng Tao  writes:
>>>>
>>>>> If it's a busy system, some times when we start an isoc transfer,
>>>>> the framenumber get from the event buffer may be already elasped,
>>>>> in this case, we will get all the packets dropped due to miss isoc.
>>>>> And we turn into transfer nothing, to fix this issue, we need to
>>>>> fix the framenumber to make sure that it's not out of date.
>>>>>
>>>>> Signed-off-by: Liang Shengjun 
>>>>> Signed-off-by: Zeng Tao 
>>>> There's a patch going upstream already doing this:
>>>>
>>>>
>https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/commit
>>>> /?h
>>>> =next=d53701067f048b8b11635e964b6d3bd9a248c622
>>>>
>>> Sorry, I think I missed to update to the latest version. But I think
>>> my patch is more efficient. Because I just sync the frame from the
>>> HW, it  won't fail and there is no need to extra tries.
>>>
>>> What do you think about it?
>> the 14 bits you use for your check is not representative of the actual
>> micro-frame you're currently in. Thinh explained that in the
>> discussion we had until we came to the patch I pointed you to above.
>> Please look at the mailing list archives for details.
>>
>
>There are several issues with this patch.
>1) Your frame elapsed time check is not based on interval but statically
>DWC3_EVENT_PRAM_SOFFN / 2. That's about 1 second. So it doesn't
>account for isoc transfers with large service interval of 1 sec or more.


This is just for checking whether the Frame number is overflow or not. So
1 second should a reason value.

>2) This function __dwc3_gadget_target_frame_elapsed() should have
>comments for what it does. The name implies that this function checks
>for eframe > cframe, and not eframe > cframe + 1s.
eframe > cframe + 1s is used to deal with the Frame number overflow.

>3) If this check fails, then it will do DWC3_ALIGN_FRAME() at least twice.
>The isoc transfer will start 1 more interval into the future than it needs to.
>
If the interval is one, 1 more interval should be more reasonable because the
core always fetch the trb one frame ahead.

>Also, I think the role of this check should be from the controller as it has
>more information and its own logic to decide if the selected future uframe
>has elapsed.

Yes, agree, but I think if the sw can be used to do the same thing and more
effective, Why not? 

BR
Zengtao 

RE: [PATCH] usb: dwc3: gadget: fix miss isoc issue introduced by IRQ latency

[Zengtao (B)]

>-Original Message-
>From: Felipe Balbi [mailto:ba...@kernel.org]
>Sent: Friday, December 14, 2018 4:52 PM
>To: Zengtao (B) 
>Cc: liangshengjun ; Zengtao (B)
>; Greg Kroah-Hartman
>; linux-...@vger.kernel.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [PATCH] usb: dwc3: gadget: fix miss isoc issue introduced by
>IRQ latency
>
>* PGP Signed by an unknown key
>
>Zeng Tao  writes:
>
>> If it's a busy system, some times when we start an isoc transfer, the
>> framenumber get from the event buffer may be already elasped, in this
>> case, we will get all the packets dropped due to miss isoc. And we
>> turn into transfer nothing, to fix this issue, we need to fix the
>> framenumber to make sure that it's not out of date.
>>
>> Signed-off-by: Liang Shengjun 
>> Signed-off-by: Zeng Tao 
>
>There's a patch going upstream already doing this:
>
>https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git/commit/?h
>=next=d53701067f048b8b11635e964b6d3bd9a248c622
>

Sorry, I think I missed to update to the latest version. But I think my 
patch is more efficient. Because I just sync the frame from the HW, it
 won't fail and there is no need to extra tries.

What do you think about it?

Regards
Zengtao
>--
>balbi
>
>* Unknown Key
>* 0xE11A9906

RE: scsi_set_medium_removal timeout issue

[Zengtao (B)]

Hi Alan:

>-Original Message-
>From: Alan Stern [mailto:st...@rowland.harvard.edu]
>Sent: Monday, November 12, 2018 11:33 PM
>To: Zengtao (B) 
>Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>usb-stor...@lists.one-eyed-alien.net
>Subject: RE: scsi_set_medium_removal timeout issue
>
>On Mon, 12 Nov 2018, Zengtao (B) wrote:
>
>> >> >Something is wrong here.  Before sending PREVENT-ALLOW
>MEDIUM
>> >> >REMOVAL, the host should issue SYNCHRONIZE CACHE.  This will
>force
>> >> >fsg_lun_fsync_sub to run, and the host should allow a long timeout
>> >> >for this command.  Then when PREVENT-ALLOW MEDIUM
>REMOVAL
>> >is sent,
>> >> >nothing will need to be flushed.
>> >> >
>> >>
>> >> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it
>> >> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe
>> >something
>> >> wrong with the scsi layer or something wrong with the mass storage
>> >class driver?
>> >
>> >Or it could be something else.  Can you please post the dmesg log
>> >from the host, showing what happens when the device is first plugged
>in?
>> >
>>
>> I have enabled the SCSI log for the host, please refer to the attachment.
>
>The log you attached was incomplete -- it was missing some commands

I just enabled the scsi log in the middle of the umount operation, otherwise I 
can't
reproduce the issue when the scsi log is enabled.

>from the beginning.  In any case, it wasn't what I wanted.  I asked you to
>post the dmesg log, not the SCSI log.

Please refer to the new attachment for dmesg log.

Thanks 
Zengtao 

~ # dmesg 
Booting Linux on physical CPU 0x0
Linux version 4.9.37 (lpcheng@osdrv) (gcc version 6.3.0 (HC 
V100R002C00B021_20180917) ) #5 SMP Mon Nov 12 19:35:04 HKT 2018
CPU: ARMv7 Processor [410fd034] revision 4 (ARMv7), cr=10c5383d
CPU: div instructions available: patching division code
CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
OF: fdt:Machine model: Hisilicon Hi3519AV100 SMP Board
cma: dma_contiguous_reserve(limit )
cma: dma_contiguous_reserve: reserving 16 MiB for global area
cma: cma_declare_contiguous(size 0x0100, base 0x, limit 0x 
alignment 0x)
cma: Reserved 16 MiB at 0x3100
Memory policy: Data cache writealloc
On node 0 totalpages: 65536
free_area_init_node: node 0, pgdat c092d580, node_mem_map cedf1000
  Normal zone: 512 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 65536 pages, LIFO batch:15
percpu: Embedded 13 pages/cpu @cedc6000 s22028 r8192 d23028 u53248
pcpu-alloc: s22028 r8192 d23028 u53248 alloc=13*4096
pcpu-alloc: [0] 0 [0] 1 
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 65024
Kernel command line: mem=256M console=ttyAMA0,115200 clk_ignore_unused 
root=/dev/mtdblock2 rw rootfstype=yaffs2 
mtdparts=hinand:1M(boot),4M(kernel),60M(rootfs) nosmp
PID hash table entries: 1024 (order: 0, 4096 bytes)
Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
Memory: 234544K/262144K available (5120K kernel code, 184K rwdata, 1368K 
rodata, 1024K init, 321K bss, 11216K reserved, 16384K cma-reserved, 0K highmem)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xd080 - 0xff80   ( 752 MB)
lowmem  : 0xc000 - 0xd000   ( 256 MB)
pkmap   : 0xbfe0 - 0xc000   (   2 MB)
modules : 0xbf00 - 0xbfe0   (  14 MB)
  .text : 0xc0008000 - 0xc060   (6112 kB)
  .init : 0xc080 - 0xc090   (1024 kB)
  .data : 0xc090 - 0xc092e180   ( 185 kB)
   .bss : 0xc093 - 0xc098072c   ( 322 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
Hierarchical RCU implementation.
Build-time adjustment of leaf fanout to 32.
RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
NR_IRQS:16 nr_irqs:16 16
arm_arch_timer: Architected cp15 timer(s) running at 24.00MHz (phys).
clocksource: arch_sys_counter: mask: 0xff max_cycles: 0x588fe9dc0, 
max_idle_ns: 440795202592 ns
sched_clock: 56 bits at 24MHz, resolution 41ns, wraps every 4398046511097ns
Switching to timer-based delay loop, resolution 41ns
clocksource: hisp804: mask: 0x max_cycles: 0x, max_idle_ns: 
637086815595 ns
Console: colour dummy device 80x30
Calibrating delay loop (skipped), value calculated using timer frequency.. 
48.00 BogoMIPS (lpj=12)
pid_max: default: 32768 minimum: 301
Mount-cache 

RE: scsi_set_medium_removal timeout issue

[Zengtao (B)]

Hi Alan:

>-Original Message-
>From: Alan Stern [mailto:st...@rowland.harvard.edu]
>Sent: Monday, November 12, 2018 11:33 PM
>To: Zengtao (B) 
>Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>usb-stor...@lists.one-eyed-alien.net
>Subject: RE: scsi_set_medium_removal timeout issue
>
>On Mon, 12 Nov 2018, Zengtao (B) wrote:
>
>> >> >Something is wrong here.  Before sending PREVENT-ALLOW
>MEDIUM
>> >> >REMOVAL, the host should issue SYNCHRONIZE CACHE.  This will
>force
>> >> >fsg_lun_fsync_sub to run, and the host should allow a long timeout
>> >> >for this command.  Then when PREVENT-ALLOW MEDIUM
>REMOVAL
>> >is sent,
>> >> >nothing will need to be flushed.
>> >> >
>> >>
>> >> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it
>> >> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe
>> >something
>> >> wrong with the scsi layer or something wrong with the mass storage
>> >class driver?
>> >
>> >Or it could be something else.  Can you please post the dmesg log
>> >from the host, showing what happens when the device is first plugged
>in?
>> >
>>
>> I have enabled the SCSI log for the host, please refer to the attachment.
>
>The log you attached was incomplete -- it was missing some commands

I just enabled the scsi log in the middle of the umount operation, otherwise I 
can't
reproduce the issue when the scsi log is enabled.

>from the beginning.  In any case, it wasn't what I wanted.  I asked you to
>post the dmesg log, not the SCSI log.

Please refer to the new attachment for dmesg log.

Thanks 
Zengtao 

~ # dmesg 
Booting Linux on physical CPU 0x0
Linux version 4.9.37 (lpcheng@osdrv) (gcc version 6.3.0 (HC 
V100R002C00B021_20180917) ) #5 SMP Mon Nov 12 19:35:04 HKT 2018
CPU: ARMv7 Processor [410fd034] revision 4 (ARMv7), cr=10c5383d
CPU: div instructions available: patching division code
CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache
OF: fdt:Machine model: Hisilicon Hi3519AV100 SMP Board
cma: dma_contiguous_reserve(limit )
cma: dma_contiguous_reserve: reserving 16 MiB for global area
cma: cma_declare_contiguous(size 0x0100, base 0x, limit 0x 
alignment 0x)
cma: Reserved 16 MiB at 0x3100
Memory policy: Data cache writealloc
On node 0 totalpages: 65536
free_area_init_node: node 0, pgdat c092d580, node_mem_map cedf1000
  Normal zone: 512 pages used for memmap
  Normal zone: 0 pages reserved
  Normal zone: 65536 pages, LIFO batch:15
percpu: Embedded 13 pages/cpu @cedc6000 s22028 r8192 d23028 u53248
pcpu-alloc: s22028 r8192 d23028 u53248 alloc=13*4096
pcpu-alloc: [0] 0 [0] 1 
Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 65024
Kernel command line: mem=256M console=ttyAMA0,115200 clk_ignore_unused 
root=/dev/mtdblock2 rw rootfstype=yaffs2 
mtdparts=hinand:1M(boot),4M(kernel),60M(rootfs) nosmp
PID hash table entries: 1024 (order: 0, 4096 bytes)
Dentry cache hash table entries: 32768 (order: 5, 131072 bytes)
Inode-cache hash table entries: 16384 (order: 4, 65536 bytes)
Memory: 234544K/262144K available (5120K kernel code, 184K rwdata, 1368K 
rodata, 1024K init, 321K bss, 11216K reserved, 16384K cma-reserved, 0K highmem)
Virtual kernel memory layout:
vector  : 0x - 0x1000   (   4 kB)
fixmap  : 0xffc0 - 0xfff0   (3072 kB)
vmalloc : 0xd080 - 0xff80   ( 752 MB)
lowmem  : 0xc000 - 0xd000   ( 256 MB)
pkmap   : 0xbfe0 - 0xc000   (   2 MB)
modules : 0xbf00 - 0xbfe0   (  14 MB)
  .text : 0xc0008000 - 0xc060   (6112 kB)
  .init : 0xc080 - 0xc090   (1024 kB)
  .data : 0xc090 - 0xc092e180   ( 185 kB)
   .bss : 0xc093 - 0xc098072c   ( 322 kB)
SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=2, Nodes=1
Hierarchical RCU implementation.
Build-time adjustment of leaf fanout to 32.
RCU restricting CPUs from NR_CPUS=4 to nr_cpu_ids=2.
RCU: Adjusting geometry for rcu_fanout_leaf=32, nr_cpu_ids=2
NR_IRQS:16 nr_irqs:16 16
arm_arch_timer: Architected cp15 timer(s) running at 24.00MHz (phys).
clocksource: arch_sys_counter: mask: 0xff max_cycles: 0x588fe9dc0, 
max_idle_ns: 440795202592 ns
sched_clock: 56 bits at 24MHz, resolution 41ns, wraps every 4398046511097ns
Switching to timer-based delay loop, resolution 41ns
clocksource: hisp804: mask: 0x max_cycles: 0x, max_idle_ns: 
637086815595 ns
Console: colour dummy device 80x30
Calibrating delay loop (skipped), value calculated using timer frequency.. 
48.00 BogoMIPS (lpj=12)
pid_max: default: 32768 minimum: 301
Mount-cache 

RE: scsi_set_medium_removal timeout issue

[Zengtao (B)]

Hi Alan:

>-Original Message-
>From: Alan Stern [mailto:st...@rowland.harvard.edu]
>Sent: Wednesday, October 31, 2018 10:20 PM
>To: Zengtao (B) 
>Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>usb-stor...@lists.one-eyed-alien.net
>Subject: RE: scsi_set_medium_removal timeout issue
>
>On Wed, 31 Oct 2018, Zengtao (B) wrote:
>
>> Hi:
>>
>> >-Original Message-
>> >From: Alan Stern [mailto:st...@rowland.harvard.edu]
>> >Sent: Tuesday, October 30, 2018 10:08 PM
>> >To: Zengtao (B) 
>> >Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>> >gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>> >linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>> >usb-stor...@lists.one-eyed-alien.net
>> >Subject: Re: scsi_set_medium_removal timeout issue
>> >
>> >On Tue, 30 Oct 2018, Zengtao (B) wrote:
>> >
>> >> Hi
>> >>
>> >> I have recently met a scsi_set_medium_removal timeout issue, and
>> >> it's related  to both SCSI and USB MASS storage.
>> >> Since i am not an expert in either scsi or usb mass storage, i am
>> >> writing to report the issue and ask for a solution from you guys.
>> >>
>> >> My test scenario is as follow:
>> >> 1.Linux HOST-Linux mass storage gadget(the back store is a
>> >> flash
>> >partition).
>> >> 2.Host mount the device.
>> >> 3.Host writes some data to the Mass storage device.
>> >> 4.Host Unmount the device.
>> >> Both Linux kernels(Host and Device) are Linux 4.9.
>> >> Some has reported the same issue a long time ago, but it remains
>there.
>> >> https://www.spinics.net/lists/linux-usb/msg53739.html
>> >>
>> >> For the issue itself, there is my observation:
>> >> In the step 4, the Host will issue an
>> >PREVENT_ALLOW_MEDIUM_REMOVAL scsi command.
>> >> and and timeout happens due to the device 's very slow
>> >fsg_lun_fsync_sub.
>> >
>> >Something is wrong here.  Before sending PREVENT-ALLOW MEDIUM
>> >REMOVAL, the host should issue SYNCHRONIZE CACHE.  This will force
>> >fsg_lun_fsync_sub to run, and the host should allow a long timeout
>> >for this command.  Then when PREVENT-ALLOW MEDIUM REMOVAL
>is sent,
>> >nothing will need to be flushed.
>> >
>>
>> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it
>> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe
>something
>> wrong with the scsi layer or something wrong with the mass storage
>class driver?
>
>Or it could be something else.  Can you please post the dmesg log from
>the host, showing what happens when the device is first plugged in?
>

I have enabled the SCSI log for the host, please refer to the attachment.

Thanks.
Zengtao 
~ # 
~ # 
~ # 
~ # 
~ #  mkfs.vfat /dev/sda1
 moun~ # 
~ #  mount /dev/sda1 /mnt

~ # 
~ #  time dd if=/dev/zero of=/mnt/test0 bs=16k count=5120

5120+0 records in
5120+0 records out
83886080 bytes (80.0MB) copied, 2.970369 seconds, 26.9MB/s
real0m 2.97s
user0m 0.01s
sys 0m 0.76s
~ # 
~ # umount /mnt

sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 12 f6 00 00 80 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b700
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] ta

RE: scsi_set_medium_removal timeout issue

[Zengtao (B)]

Hi Alan:

>-Original Message-
>From: Alan Stern [mailto:st...@rowland.harvard.edu]
>Sent: Wednesday, October 31, 2018 10:20 PM
>To: Zengtao (B) 
>Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>usb-stor...@lists.one-eyed-alien.net
>Subject: RE: scsi_set_medium_removal timeout issue
>
>On Wed, 31 Oct 2018, Zengtao (B) wrote:
>
>> Hi:
>>
>> >-Original Message-
>> >From: Alan Stern [mailto:st...@rowland.harvard.edu]
>> >Sent: Tuesday, October 30, 2018 10:08 PM
>> >To: Zengtao (B) 
>> >Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>> >gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>> >linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>> >usb-stor...@lists.one-eyed-alien.net
>> >Subject: Re: scsi_set_medium_removal timeout issue
>> >
>> >On Tue, 30 Oct 2018, Zengtao (B) wrote:
>> >
>> >> Hi
>> >>
>> >> I have recently met a scsi_set_medium_removal timeout issue, and
>> >> it's related  to both SCSI and USB MASS storage.
>> >> Since i am not an expert in either scsi or usb mass storage, i am
>> >> writing to report the issue and ask for a solution from you guys.
>> >>
>> >> My test scenario is as follow:
>> >> 1.Linux HOST-Linux mass storage gadget(the back store is a
>> >> flash
>> >partition).
>> >> 2.Host mount the device.
>> >> 3.Host writes some data to the Mass storage device.
>> >> 4.Host Unmount the device.
>> >> Both Linux kernels(Host and Device) are Linux 4.9.
>> >> Some has reported the same issue a long time ago, but it remains
>there.
>> >> https://www.spinics.net/lists/linux-usb/msg53739.html
>> >>
>> >> For the issue itself, there is my observation:
>> >> In the step 4, the Host will issue an
>> >PREVENT_ALLOW_MEDIUM_REMOVAL scsi command.
>> >> and and timeout happens due to the device 's very slow
>> >fsg_lun_fsync_sub.
>> >
>> >Something is wrong here.  Before sending PREVENT-ALLOW MEDIUM
>> >REMOVAL, the host should issue SYNCHRONIZE CACHE.  This will force
>> >fsg_lun_fsync_sub to run, and the host should allow a long timeout
>> >for this command.  Then when PREVENT-ALLOW MEDIUM REMOVAL
>is sent,
>> >nothing will need to be flushed.
>> >
>>
>> Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it
>> directly issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe
>something
>> wrong with the scsi layer or something wrong with the mass storage
>class driver?
>
>Or it could be something else.  Can you please post the dmesg log from
>the host, showing what happens when the device is first plugged in?
>

I have enabled the SCSI log for the host, please refer to the attachment.

Thanks.
Zengtao 
~ # 
~ # 
~ # 
~ # 
~ #  mkfs.vfat /dev/sda1
 moun~ # 
~ #  mount /dev/sda1 /mnt

~ # 
~ #  time dd if=/dev/zero of=/mnt/test0 bs=16k count=5120

5120+0 records in
5120+0 records out
83886080 bytes (80.0MB) copied, 2.970369 seconds, 26.9MB/s
real0m 2.97s
user0m 0.01s
sys 0m 0.76s
~ # 
~ # umount /mnt

sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 12 f6 00 00 80 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 13 76 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b700
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 14 66 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] tag#0 Send: scmd 0xce13b800
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 Done: SUCCESS Result: hostbyte=DID_OK 
driverbyte=DRIVER_OK
sd 0:0:0:0: [sda] tag#0 CDB: Write(10) 2a 00 00 02 15 56 00 00 f0 00
sd 0:0:0:0: [sda] tag#0 scsi host busy 1 failed 0
sd 0:0:0:0: Notifying upper driver of completion (result 0)
sd 0:0:0:0: [sda] ta

RE: scsi_set_medium_removal timeout issue

[Zengtao (B)]

Hi:

>-Original Message-
>From: Alan Stern [mailto:st...@rowland.harvard.edu]
>Sent: Tuesday, October 30, 2018 10:08 PM
>To: Zengtao (B) 
>Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>usb-stor...@lists.one-eyed-alien.net
>Subject: Re: scsi_set_medium_removal timeout issue
>
>On Tue, 30 Oct 2018, Zengtao (B) wrote:
>
>> Hi
>>
>> I have recently met a scsi_set_medium_removal timeout issue, and it's
>> related  to both SCSI and USB MASS storage.
>> Since i am not an expert in either scsi or usb mass storage, i am
>> writing to report the issue and ask for a solution from you guys.
>>
>> My test scenario is as follow:
>> 1.Linux HOST-Linux mass storage gadget(the back store is a flash
>partition).
>> 2.Host mount the device.
>> 3.Host writes some data to the Mass storage device.
>> 4.Host Unmount the device.
>> Both Linux kernels(Host and Device) are Linux 4.9.
>> Some has reported the same issue a long time ago, but it remains there.
>> https://www.spinics.net/lists/linux-usb/msg53739.html
>>
>> For the issue itself, there is my observation:
>> In the step 4, the Host will issue an
>PREVENT_ALLOW_MEDIUM_REMOVAL scsi command.
>> and and timeout happens due to the device 's very slow
>fsg_lun_fsync_sub.
>
>Something is wrong here.  Before sending PREVENT-ALLOW MEDIUM
>REMOVAL, the host should issue SYNCHRONIZE CACHE.  This will force
>fsg_lun_fsync_sub to run, and the host should allow a long timeout for
>this command.  Then when PREVENT-ALLOW MEDIUM REMOVAL is sent,
>nothing will need to be flushed.
>

Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it directly
issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe something wrong
with the scsi layer or something wrong with the mass storage class driver?

Zengtao 

>Alan Stern
>
>> I found there are two methods to workaround the issue:
>> 1. Change the timeout value of host scsi command
>> PREVENT_ALLOW_MEDIUM_REMOVAL to to about 60 seconds from 10
>seconds.
>> 2. Remove the fsg_lun_fsync_sub in the device's Mass storage gadget
>driver.
>>
>> Thanks
>>
>> Regards
>> zentao

RE: scsi_set_medium_removal timeout issue

[Zengtao (B)]

Hi:

>-Original Message-
>From: Alan Stern [mailto:st...@rowland.harvard.edu]
>Sent: Tuesday, October 30, 2018 10:08 PM
>To: Zengtao (B) 
>Cc: j...@linux.vnet.ibm.com; martin.peter...@oracle.com;
>gre...@linuxfoundation.org; linux-s...@vger.kernel.org;
>linux-kernel@vger.kernel.org; linux-...@vger.kernel.org;
>usb-stor...@lists.one-eyed-alien.net
>Subject: Re: scsi_set_medium_removal timeout issue
>
>On Tue, 30 Oct 2018, Zengtao (B) wrote:
>
>> Hi
>>
>> I have recently met a scsi_set_medium_removal timeout issue, and it's
>> related  to both SCSI and USB MASS storage.
>> Since i am not an expert in either scsi or usb mass storage, i am
>> writing to report the issue and ask for a solution from you guys.
>>
>> My test scenario is as follow:
>> 1.Linux HOST-Linux mass storage gadget(the back store is a flash
>partition).
>> 2.Host mount the device.
>> 3.Host writes some data to the Mass storage device.
>> 4.Host Unmount the device.
>> Both Linux kernels(Host and Device) are Linux 4.9.
>> Some has reported the same issue a long time ago, but it remains there.
>> https://www.spinics.net/lists/linux-usb/msg53739.html
>>
>> For the issue itself, there is my observation:
>> In the step 4, the Host will issue an
>PREVENT_ALLOW_MEDIUM_REMOVAL scsi command.
>> and and timeout happens due to the device 's very slow
>fsg_lun_fsync_sub.
>
>Something is wrong here.  Before sending PREVENT-ALLOW MEDIUM
>REMOVAL, the host should issue SYNCHRONIZE CACHE.  This will force
>fsg_lun_fsync_sub to run, and the host should allow a long timeout for
>this command.  Then when PREVENT-ALLOW MEDIUM REMOVAL is sent,
>nothing will need to be flushed.
>

Definitely, I haven't seen the SYNCHRONIZE CACHE from the host, it directly
issued the PREVENT-ALLOW MEDIUM REMOVAL, so maybe something wrong
with the scsi layer or something wrong with the mass storage class driver?

Zengtao 

>Alan Stern
>
>> I found there are two methods to workaround the issue:
>> 1. Change the timeout value of host scsi command
>> PREVENT_ALLOW_MEDIUM_REMOVAL to to about 60 seconds from 10
>seconds.
>> 2. Remove the fsg_lun_fsync_sub in the device's Mass storage gadget
>driver.
>>
>> Thanks
>>
>> Regards
>> zentao

RE: [RESEND PATCH] tee: add kernel internal client interface

[Zengtao (B)]

Hi jens:

Actually, we have already used the kernel client api in our product(poplar 
board).
Thank you for the upstream.

Tested-by: Zeng Tao 

Regards
Zengtao 

>-Original Message-
>From: Jens Wiklander [mailto:jens.wiklan...@linaro.org]
>Sent: Monday, July 09, 2018 2:16 PM
>To: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
>tee-...@lists.linaro.org
>Cc: Zengtao (B) ; Victor Chong
>; Jerome Forissier ;
>Jens Wiklander 
>Subject: [RESEND PATCH] tee: add kernel internal client interface
>
>Adds a kernel internal TEE client interface to be used by other drivers.
>
>Signed-off-by: Jens Wiklander 
>---
> drivers/tee/tee_core.c  | 113 +---
> include/linux/tee_drv.h |  73 ++
> 2 files changed, 179 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index
>dd46b758852a..7b2bb4c50058 100644
>--- a/drivers/tee/tee_core.c
>+++ b/drivers/tee/tee_core.c
>@@ -38,15 +38,13 @@ static DEFINE_SPINLOCK(driver_lock);  static struct
>class *tee_class;  static dev_t tee_devt;
>
>-static int tee_open(struct inode *inode, struct file *filp)
>+static struct tee_context *teedev_open(struct tee_device *teedev)
> {
>   int rc;
>-  struct tee_device *teedev;
>   struct tee_context *ctx;
>
>-  teedev = container_of(inode->i_cdev, struct tee_device, cdev);
>   if (!tee_device_get(teedev))
>-  return -EINVAL;
>+  return ERR_PTR(-EINVAL);
>
>   ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>   if (!ctx) {
>@@ -57,16 +55,16 @@ static int tee_open(struct inode *inode, struct file *filp)
>   kref_init(>refcount);
>   ctx->teedev = teedev;
>   INIT_LIST_HEAD(>list_shm);
>-  filp->private_data = ctx;
>   rc = teedev->desc->ops->open(ctx);
>   if (rc)
>   goto err;
>
>-  return 0;
>+  return ctx;
> err:
>   kfree(ctx);
>   tee_device_put(teedev);
>-  return rc;
>+  return ERR_PTR(rc);
>+
> }
>
> void teedev_ctx_get(struct tee_context *ctx) @@ -100,6 +98,18 @@ static
>void teedev_close_context(struct tee_context *ctx)
>   teedev_ctx_put(ctx);
> }
>
>+static int tee_open(struct inode *inode, struct file *filp) {
>+  struct tee_context *ctx;
>+
>+  ctx = teedev_open(container_of(inode->i_cdev, struct tee_device, cdev));
>+  if (IS_ERR(ctx))
>+  return PTR_ERR(ctx);
>+
>+  filp->private_data = ctx;
>+  return 0;
>+}
>+
> static int tee_release(struct inode *inode, struct file *filp)  {
>   teedev_close_context(filp->private_data);
>@@ -928,6 +938,95 @@ void *tee_get_drvdata(struct tee_device *teedev)  }
>EXPORT_SYMBOL_GPL(tee_get_drvdata);
>
>+struct match_dev_data {
>+  struct tee_ioctl_version_data *vers;
>+  const void *data;
>+  int (*match)(struct tee_ioctl_version_data *, const void *); };
>+
>+static int match_dev(struct device *dev, const void *data) {
>+  const struct match_dev_data *match_data = data;
>+  struct tee_device *teedev = container_of(dev, struct tee_device, dev);
>+
>+  teedev->desc->ops->get_version(teedev, match_data->vers);
>+  return match_data->match(match_data->vers, match_data->data); }
>+
>+struct tee_context *
>+tee_client_open_context(struct tee_context *start,
>+  int (*match)(struct tee_ioctl_version_data *,
>+   const void *),
>+  const void *data, struct tee_ioctl_version_data *vers) {
>+  struct device *dev = NULL;
>+  struct device *put_dev = NULL;
>+  struct tee_context *ctx = NULL;
>+  struct tee_ioctl_version_data v;
>+  struct match_dev_data match_data = { vers ? vers : , data, match };
>+
>+  if (start)
>+  dev = >teedev->dev;
>+
>+  do {
>+  dev = class_find_device(tee_class, dev, _data, match_dev);
>+  if (!dev) {
>+  ctx = ERR_PTR(-ENOENT);
>+  break;
>+  }
>+
>+  put_device(put_dev);
>+  put_dev = dev;
>+
>+  ctx = teedev_open(container_of(dev, struct tee_device, dev));
>+  } while (IS_ERR(ctx) && PTR_ERR(ctx) != -ENOMEM);
>+
>+  put_device(put_dev);
>+  return ctx;
>+}
>+EXPORT_SYMBOL_GPL(tee_client_open_context);
>+
>+void tee_client_close_context(struct tee_context *ctx) {
>+  teedev_close_context(ctx);
>+}
>+EXPORT_SYMBOL_GPL(tee_client_close_context);
>+
>+void tee_client_get_version(struct tee_context *ctx,
>

RE: [RESEND PATCH] tee: add kernel internal client interface

[Zengtao (B)]

Hi jens:

Actually, we have already used the kernel client api in our product(poplar 
board).
Thank you for the upstream.

Tested-by: Zeng Tao 

Regards
Zengtao 

>-Original Message-
>From: Jens Wiklander [mailto:jens.wiklan...@linaro.org]
>Sent: Monday, July 09, 2018 2:16 PM
>To: linux-kernel@vger.kernel.org; linux-arm-ker...@lists.infradead.org;
>tee-...@lists.linaro.org
>Cc: Zengtao (B) ; Victor Chong
>; Jerome Forissier ;
>Jens Wiklander 
>Subject: [RESEND PATCH] tee: add kernel internal client interface
>
>Adds a kernel internal TEE client interface to be used by other drivers.
>
>Signed-off-by: Jens Wiklander 
>---
> drivers/tee/tee_core.c  | 113 +---
> include/linux/tee_drv.h |  73 ++
> 2 files changed, 179 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index
>dd46b758852a..7b2bb4c50058 100644
>--- a/drivers/tee/tee_core.c
>+++ b/drivers/tee/tee_core.c
>@@ -38,15 +38,13 @@ static DEFINE_SPINLOCK(driver_lock);  static struct
>class *tee_class;  static dev_t tee_devt;
>
>-static int tee_open(struct inode *inode, struct file *filp)
>+static struct tee_context *teedev_open(struct tee_device *teedev)
> {
>   int rc;
>-  struct tee_device *teedev;
>   struct tee_context *ctx;
>
>-  teedev = container_of(inode->i_cdev, struct tee_device, cdev);
>   if (!tee_device_get(teedev))
>-  return -EINVAL;
>+  return ERR_PTR(-EINVAL);
>
>   ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
>   if (!ctx) {
>@@ -57,16 +55,16 @@ static int tee_open(struct inode *inode, struct file *filp)
>   kref_init(>refcount);
>   ctx->teedev = teedev;
>   INIT_LIST_HEAD(>list_shm);
>-  filp->private_data = ctx;
>   rc = teedev->desc->ops->open(ctx);
>   if (rc)
>   goto err;
>
>-  return 0;
>+  return ctx;
> err:
>   kfree(ctx);
>   tee_device_put(teedev);
>-  return rc;
>+  return ERR_PTR(rc);
>+
> }
>
> void teedev_ctx_get(struct tee_context *ctx) @@ -100,6 +98,18 @@ static
>void teedev_close_context(struct tee_context *ctx)
>   teedev_ctx_put(ctx);
> }
>
>+static int tee_open(struct inode *inode, struct file *filp) {
>+  struct tee_context *ctx;
>+
>+  ctx = teedev_open(container_of(inode->i_cdev, struct tee_device, cdev));
>+  if (IS_ERR(ctx))
>+  return PTR_ERR(ctx);
>+
>+  filp->private_data = ctx;
>+  return 0;
>+}
>+
> static int tee_release(struct inode *inode, struct file *filp)  {
>   teedev_close_context(filp->private_data);
>@@ -928,6 +938,95 @@ void *tee_get_drvdata(struct tee_device *teedev)  }
>EXPORT_SYMBOL_GPL(tee_get_drvdata);
>
>+struct match_dev_data {
>+  struct tee_ioctl_version_data *vers;
>+  const void *data;
>+  int (*match)(struct tee_ioctl_version_data *, const void *); };
>+
>+static int match_dev(struct device *dev, const void *data) {
>+  const struct match_dev_data *match_data = data;
>+  struct tee_device *teedev = container_of(dev, struct tee_device, dev);
>+
>+  teedev->desc->ops->get_version(teedev, match_data->vers);
>+  return match_data->match(match_data->vers, match_data->data); }
>+
>+struct tee_context *
>+tee_client_open_context(struct tee_context *start,
>+  int (*match)(struct tee_ioctl_version_data *,
>+   const void *),
>+  const void *data, struct tee_ioctl_version_data *vers) {
>+  struct device *dev = NULL;
>+  struct device *put_dev = NULL;
>+  struct tee_context *ctx = NULL;
>+  struct tee_ioctl_version_data v;
>+  struct match_dev_data match_data = { vers ? vers : , data, match };
>+
>+  if (start)
>+  dev = >teedev->dev;
>+
>+  do {
>+  dev = class_find_device(tee_class, dev, _data, match_dev);
>+  if (!dev) {
>+  ctx = ERR_PTR(-ENOENT);
>+  break;
>+  }
>+
>+  put_device(put_dev);
>+  put_dev = dev;
>+
>+  ctx = teedev_open(container_of(dev, struct tee_device, dev));
>+  } while (IS_ERR(ctx) && PTR_ERR(ctx) != -ENOMEM);
>+
>+  put_device(put_dev);
>+  return ctx;
>+}
>+EXPORT_SYMBOL_GPL(tee_client_open_context);
>+
>+void tee_client_close_context(struct tee_context *ctx) {
>+  teedev_close_context(ctx);
>+}
>+EXPORT_SYMBOL_GPL(tee_client_close_context);
>+
>+void tee_client_get_version(struct tee_context *ctx,
>

RE: [alsa-devel] Timeout issues in wait_for_avail function

[Zengtao (B)]

Hi Takashi:

Thank you for your reply.

>-Original Message-
>From: Takashi Iwai [mailto:ti...@suse.de]
>Sent: Sunday, May 13, 2018 6:40 PM
>To: Zengtao (B) 
>Cc: pe...@perex.cz; ti...@suse.com; alsa-de...@alsa-project.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [alsa-devel] Timeout issues in wait_for_avail function
>
>On Mon, 07 May 2018 12:49:34 +0200,
>Zengtao (B) wrote:
>>
>> Hi perex and tiwai:
>>
>> I have met a timeout case when capture audio from snd-usb-audio
>> device, when the host call the pcm_read and get into the wait_for_avail
>function.
>> The following happends
>> 1.  No available data for capture(maybe because of the late response
>> audio data by the uac device)
>
>Hrm, in the case of capture, the data must be available.
>If it's not the case, something is wrong.
>
>> 2.  The current thread falls into sleep state and no one wakes up it.
>> 3.  The current thread will sleep 10s(schedule_timeout(1000)) and then
>wakeup.
>>
>> I have two question about the wait_for_avail:
>> 1. The timeout value too long, is it a reasonable value?
>> 2. Is there any mechanism to wake up the thread if there is data from the
>hw.
>
>The scenario above shouldn't happen, so no need for discussion.
>Rather we should check why it's woken up even though no data is available.
>

It really happens on my platform, and anyway 10 seconds timeout seems
not a reasonable value. And I don't there is any guarantee that there must be 
avail
data when we reach wait_for_avail, in fact, inside the wait_for_avail, there is 
branch
when no data is available.

>You can check the tracepoints to see the action of PCM stream, and confirm
>whether it's really no data, or it's just lost by some reason (or looks as if 
>so).
>
I trace the data flow, we reach the wait_for_avail function before the usb snd 
data arrives, 
so we wait until 10 seconds timeout. 

>
>Takashi


Regards
Zengtao 

RE: [alsa-devel] Timeout issues in wait_for_avail function

[Zengtao (B)]

Hi Takashi:

Thank you for your reply.

>-Original Message-
>From: Takashi Iwai [mailto:ti...@suse.de]
>Sent: Sunday, May 13, 2018 6:40 PM
>To: Zengtao (B) 
>Cc: pe...@perex.cz; ti...@suse.com; alsa-de...@alsa-project.org;
>linux-kernel@vger.kernel.org
>Subject: Re: [alsa-devel] Timeout issues in wait_for_avail function
>
>On Mon, 07 May 2018 12:49:34 +0200,
>Zengtao (B) wrote:
>>
>> Hi perex and tiwai:
>>
>> I have met a timeout case when capture audio from snd-usb-audio
>> device, when the host call the pcm_read and get into the wait_for_avail
>function.
>> The following happends
>> 1.  No available data for capture(maybe because of the late response
>> audio data by the uac device)
>
>Hrm, in the case of capture, the data must be available.
>If it's not the case, something is wrong.
>
>> 2.  The current thread falls into sleep state and no one wakes up it.
>> 3.  The current thread will sleep 10s(schedule_timeout(1000)) and then
>wakeup.
>>
>> I have two question about the wait_for_avail:
>> 1. The timeout value too long, is it a reasonable value?
>> 2. Is there any mechanism to wake up the thread if there is data from the
>hw.
>
>The scenario above shouldn't happen, so no need for discussion.
>Rather we should check why it's woken up even though no data is available.
>

It really happens on my platform, and anyway 10 seconds timeout seems
not a reasonable value. And I don't there is any guarantee that there must be 
avail
data when we reach wait_for_avail, in fact, inside the wait_for_avail, there is 
branch
when no data is available.

>You can check the tracepoints to see the action of PCM stream, and confirm
>whether it's really no data, or it's just lost by some reason (or looks as if 
>so).
>
I trace the data flow, we reach the wait_for_avail function before the usb snd 
data arrives, 
so we wait until 10 seconds timeout. 

>
>Takashi


Regards
Zengtao 

Timeout issues in wait_for_avail function

[Zengtao (B)]

Hi perex and tiwai:

I have met a timeout case when capture audio from snd-usb-audio device, 
when the host call the pcm_read and get into the wait_for_avail function.
The following happends
1.  No available data for capture(maybe because of the late response audio data 
by the uac device)
2.  The current thread falls into sleep state and no one wakes up it.
3.  The current thread will sleep 10s(schedule_timeout(1000)) and then wakeup.

I have two question about the wait_for_avail:
1. The timeout value too long, is it a reasonable value? 
2. Is there any mechanism to wake up the thread if there is data from the hw.

Regards
Zengtao

Timeout issues in wait_for_avail function

[Zengtao (B)]

Hi perex and tiwai:

I have met a timeout case when capture audio from snd-usb-audio device, 
when the host call the pcm_read and get into the wait_for_avail function.
The following happends
1.  No available data for capture(maybe because of the late response audio data 
by the uac device)
2.  The current thread falls into sleep state and no one wakes up it.
3.  The current thread will sleep 10s(schedule_timeout(1000)) and then wakeup.

I have two question about the wait_for_avail:
1. The timeout value too long, is it a reasonable value? 
2. Is there any mechanism to wake up the thread if there is data from the hw.

Regards
Zengtao

Possible usb_request leak in the function dwc2_gadget_complete_isoc_request_ddma

[Zengtao (B)]

Hi johnyoun:

I found a suspected bug, and I am writing to confirm with you.

In the function 
dwc2_gadget_complete_isoc_request_ddma(drivers/usb/dwc2/gadget.c).
Only the first request from the eq queue is processed while maybe there are 
more than one descriptors done by the HW.

1. Each usb request is associated with a DMA descriptor, but this is not 
reflect in the driver, so when one DMA descriptor is done, 
we don't know which usb request is done, but I think if only one DMA descriptor 
is done, we can know that the first USB request in 
eq queue is done, because the HW DMA descriptor and SW usb request are both in 
sequence.

2. In the function dwc2_gadget_complete_isoc_request_ddma, we may complete more 
than one DMA descriptor but only the first
Usb request is processed, but in fact, we should all the usb requests 
associated with all the done DMA descriptors.

3. I noticed that each DMA descriptor is configured to report an interrupt, and 
if each DMA descriptor generate an interrupt, the above
Flow should be ok, but the interrupts can merge and we have used the depdma to 
figure out the largest finished DMA descriptor index.

Looking forward your reply.

Thank you. 

Regards
Zengtao 

Possible usb_request leak in the function dwc2_gadget_complete_isoc_request_ddma

[Zengtao (B)]

Hi johnyoun:

I found a suspected bug, and I am writing to confirm with you.

In the function 
dwc2_gadget_complete_isoc_request_ddma(drivers/usb/dwc2/gadget.c).
Only the first request from the eq queue is processed while maybe there are 
more than one descriptors done by the HW.

1. Each usb request is associated with a DMA descriptor, but this is not 
reflect in the driver, so when one DMA descriptor is done, 
we don't know which usb request is done, but I think if only one DMA descriptor 
is done, we can know that the first USB request in 
eq queue is done, because the HW DMA descriptor and SW usb request are both in 
sequence.

2. In the function dwc2_gadget_complete_isoc_request_ddma, we may complete more 
than one DMA descriptor but only the first
Usb request is processed, but in fact, we should all the usb requests 
associated with all the done DMA descriptors.

3. I noticed that each DMA descriptor is configured to report an interrupt, and 
if each DMA descriptor generate an interrupt, the above
Flow should be ok, but the interrupts can merge and we have used the depdma to 
figure out the largest finished DMA descriptor index.

Looking forward your reply.

Thank you. 

Regards
Zengtao 

Using ion memory for direct-io

[Zengtao (B)]

Hi 

Currently, the ion mapped to userspace will be forced with VM_IO and VM_PFNMAP 
flags.
When I use the ion memory to do the direct-io, it will fail when reaching the 
get_user_pages,

Back to the VM_IO and VM_PFNMAP flag, there two flags are introduced by the 
remap_pfn_range called 
by the ion_heap_mmap_user.

>From my point of view, all ion memory(cma/vmalloc/system heap) are managed by 
>linux vm, it
is not reasonable to have the VM_IO and VM_PFNMAP flag, but I don't any 
suitable function
to replace the remap_pfn_range, any suggestions?

Thanks && Regards

Zengtao 

Using ion memory for direct-io

[Zengtao (B)]

Hi 

Currently, the ion mapped to userspace will be forced with VM_IO and VM_PFNMAP 
flags.
When I use the ion memory to do the direct-io, it will fail when reaching the 
get_user_pages,

Back to the VM_IO and VM_PFNMAP flag, there two flags are introduced by the 
remap_pfn_range called 
by the ion_heap_mmap_user.

>From my point of view, all ion memory(cma/vmalloc/system heap) are managed by 
>linux vm, it
is not reasonable to have the VM_IO and VM_PFNMAP flag, but I don't any 
suitable function
to replace the remap_pfn_range, any suggestions?

Thanks && Regards

Zengtao 

RE: [PATCH] arm64: fix /proc/cpuinfo for elf32

[Zengtao (B)]

> -Original Message-
> From: Suzuki K Poulose [mailto:suzuki.poul...@arm.com]
> Sent: Tuesday, April 26, 2016 5:21 PM
> To: Zengtao (B); Catalin Marinas
> Cc: will.dea...@arm.com; mark.rutl...@arm.com; yang@linaro.org;
> linux-kernel@vger.kernel.org; james.mo...@arm.com;
> linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] arm64: fix /proc/cpuinfo for elf32
> 
> On 26/04/16 03:21, Zengtao (B) wrote:
> >
> > So you mean the 64-bit grep should see the same cpuinfo as its father 
> > process
> > which is 32-bit?
> >
> > For 32-bit process running on 64-bit kernel, we have to explicitly call the
> personality
> > syscall to get the right cpuinfo, but how to deal with the old 32-bit 
> > binaries?
> 
> Yes, you could use the syscall to switch the personality. Alternately, you 
> could
> run
> "linux32" command to switch the personality to PER_LINUX32 and then
> execute
> 64/32 bit applications.
>

Any plan to place the personality syscall in libc? It is not nessary for each 
32-bit process 
to do the personality syscall or linux32 command. 
 
> Suzuki
> 
> 

RE: [PATCH] arm64: fix /proc/cpuinfo for elf32

[Zengtao (B)]

> -Original Message-
> From: Suzuki K Poulose [mailto:suzuki.poul...@arm.com]
> Sent: Tuesday, April 26, 2016 5:21 PM
> To: Zengtao (B); Catalin Marinas
> Cc: will.dea...@arm.com; mark.rutl...@arm.com; yang@linaro.org;
> linux-kernel@vger.kernel.org; james.mo...@arm.com;
> linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] arm64: fix /proc/cpuinfo for elf32
> 
> On 26/04/16 03:21, Zengtao (B) wrote:
> >
> > So you mean the 64-bit grep should see the same cpuinfo as its father 
> > process
> > which is 32-bit?
> >
> > For 32-bit process running on 64-bit kernel, we have to explicitly call the
> personality
> > syscall to get the right cpuinfo, but how to deal with the old 32-bit 
> > binaries?
> 
> Yes, you could use the syscall to switch the personality. Alternately, you 
> could
> run
> "linux32" command to switch the personality to PER_LINUX32 and then
> execute
> 64/32 bit applications.
>

Any plan to place the personality syscall in libc? It is not nessary for each 
32-bit process 
to do the personality syscall or linux32 command. 
 
> Suzuki
> 
> 

RE: [PATCH] arm64: fix /proc/cpuinfo for elf32

[Zengtao (B)]

> -Original Message-
> From: Catalin Marinas [mailto:catalin.mari...@arm.com]
> Sent: Monday, April 25, 2016 5:13 PM
> To: Zengtao (B)
> Cc: will.dea...@arm.com; mark.rutl...@arm.com; yang@linaro.org;
> suzuki.poul...@arm.com; linux-kernel@vger.kernel.org;
> james.mo...@arm.com; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] arm64: fix /proc/cpuinfo for elf32
> 
> On Mon, Apr 25, 2016 at 11:37:33AM +0800, Zeng Tao wrote:
> > For elf32 thread, personality is used for arm32,
> > and thread_flag for arm64.
> >
> > Here personality is used for arm64, so fix it.
> >
> > Signed-off-by: Zeng Tao <prime.z...@huawei.com>
> > ---
> >  arch/arm64/kernel/cpuinfo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > index 84c8684..f739398 100644
> > --- a/arch/arm64/kernel/cpuinfo.c
> > +++ b/arch/arm64/kernel/cpuinfo.c
> > @@ -126,7 +126,7 @@ static int c_show(struct seq_file *m, void *v)
> >  * software which does already (at least for 32-bit).
> >  */
> > seq_puts(m, "Features\t:");
> > -   if (personality(current->personality) == PER_LINUX32) {
> > +   if (test_thread_flag(TIF_32BIT)) {
> >  #ifdef CONFIG_COMPAT
> > for (j = 0; compat_hwcap_str[j]; j++)
> > if (compat_elf_hwcap & (1 << j))
> 
> We discussed this some time ago and we decided against it. One reason
> was scripts where you may or may not end up with the desired cpuinfo
> (e.g. grep being 64-bit invoked by a 32-bit bash). The personality at
> least is inherited by child processes.

So you mean the 64-bit grep should see the same cpuinfo as its father process
which is 32-bit?

For 32-bit process running on 64-bit kernel, we have to explicitly call the 
personality 
syscall to get the right cpuinfo, but how to deal with the old 32-bit binaries? 
 

> 
> --
> Catalin

RE: [PATCH] arm64: fix /proc/cpuinfo for elf32

[Zengtao (B)]

> -Original Message-
> From: Catalin Marinas [mailto:catalin.mari...@arm.com]
> Sent: Monday, April 25, 2016 5:13 PM
> To: Zengtao (B)
> Cc: will.dea...@arm.com; mark.rutl...@arm.com; yang@linaro.org;
> suzuki.poul...@arm.com; linux-kernel@vger.kernel.org;
> james.mo...@arm.com; linux-arm-ker...@lists.infradead.org
> Subject: Re: [PATCH] arm64: fix /proc/cpuinfo for elf32
> 
> On Mon, Apr 25, 2016 at 11:37:33AM +0800, Zeng Tao wrote:
> > For elf32 thread, personality is used for arm32,
> > and thread_flag for arm64.
> >
> > Here personality is used for arm64, so fix it.
> >
> > Signed-off-by: Zeng Tao 
> > ---
> >  arch/arm64/kernel/cpuinfo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
> > index 84c8684..f739398 100644
> > --- a/arch/arm64/kernel/cpuinfo.c
> > +++ b/arch/arm64/kernel/cpuinfo.c
> > @@ -126,7 +126,7 @@ static int c_show(struct seq_file *m, void *v)
> >  * software which does already (at least for 32-bit).
> >  */
> > seq_puts(m, "Features\t:");
> > -   if (personality(current->personality) == PER_LINUX32) {
> > +   if (test_thread_flag(TIF_32BIT)) {
> >  #ifdef CONFIG_COMPAT
> > for (j = 0; compat_hwcap_str[j]; j++)
> > if (compat_elf_hwcap & (1 << j))
> 
> We discussed this some time ago and we decided against it. One reason
> was scripts where you may or may not end up with the desired cpuinfo
> (e.g. grep being 64-bit invoked by a 32-bit bash). The personality at
> least is inherited by child processes.

So you mean the 64-bit grep should see the same cpuinfo as its father process
which is 32-bit?

For 32-bit process running on 64-bit kernel, we have to explicitly call the 
personality 
syscall to get the right cpuinfo, but how to deal with the old 32-bit binaries? 
 

> 
> --
> Catalin

Re: [PATCH] arm64: fix /proc/cpuinfo for elf32

[Zengtao (B)]

On 2016-04-25 09:12, Catalin Marinas wrote:
> On Mon, Apr 25, 2016 at 11:37:33AM +0800, Zeng Tao wrote:
> > For elf32 thread, personality is used for arm32,
> > and thread_flag for arm64.
> > 
> > Here personality is used for arm64, so fix it.
> > 
> > Signed-off-by: Zeng Tao 
> > ---
> >  arch/arm64/kernel/cpuinfo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> 
> We discussed this some time ago and we decided against it. One reason
> was scripts where you may or may not end up with the desired cpuinfo
> (e.g. grep being 64-bit invoked by a 32-bit bash). The personality at
> least is inherited by child processes.
> 
So you mean the 64-bit grep should see the same cpuinfo as 32-bit bash as 
It is the child process?
But currently we have same 32-bit application which don't have their
personality set, so they get the wrong cpuinfo. 
How to fix, call the personality syscall?  

> -- 
> Catalin
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

Re: [PATCH] arm64: fix /proc/cpuinfo for elf32

[Zengtao (B)]

On 2016-04-25 09:12, Catalin Marinas wrote:
> On Mon, Apr 25, 2016 at 11:37:33AM +0800, Zeng Tao wrote:
> > For elf32 thread, personality is used for arm32,
> > and thread_flag for arm64.
> > 
> > Here personality is used for arm64, so fix it.
> > 
> > Signed-off-by: Zeng Tao 
> > ---
> >  arch/arm64/kernel/cpuinfo.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> 
> We discussed this some time ago and we decided against it. One reason
> was scripts where you may or may not end up with the desired cpuinfo
> (e.g. grep being 64-bit invoked by a 32-bit bash). The personality at
> least is inherited by child processes.
> 
So you mean the 64-bit grep should see the same cpuinfo as 32-bit bash as 
It is the child process?
But currently we have same 32-bit application which don't have their
personality set, so they get the wrong cpuinfo. 
How to fix, call the personality syscall?  

> -- 
> Catalin
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

> -Original Message-
> From: Thomas Gleixner [mailto:t...@linutronix.de]
> Sent: Monday, February 01, 2016 4:43 PM
> To: Zengtao (B)
> Cc: Arnd Bergmann; LKML; Frederic Weisbecker
> Subject: RE: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> On Mon, 1 Feb 2016, Zengtao (B) wrote:
> 
> > Hi Arnd:
> > I have got a new idea about the problem:
> > In include/linux/time64.h
> > #define NSEC_PER_SEC10L
> > I think we should change it to
> > #define NSEC_PER_SEC10LL
> >
> > My reason is :
> > 1.  when it is used in a multiplication, it will easily get overflow.
> > 2.  when it don't get overflow, the change has no side affect.
> 
> That's not a good idea. NSEC_PER_SEC is used in lots of places with 32bit
> storage. I really don't want to mop up all the fallout.

Yes, agree, a lot of places it has been used as 32bit value. Beyond that, I 
think 
it is more reasonable to use 64bit. 

> 
> Thanks,
> 
>   tglx
> 

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

> -Original Message-
> From: Thomas Gleixner [mailto:t...@linutronix.de]
> Sent: Monday, February 01, 2016 4:43 PM
> To: Zengtao (B)
> Cc: Arnd Bergmann; LKML; Frederic Weisbecker
> Subject: RE: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> On Mon, 1 Feb 2016, Zengtao (B) wrote:
> 
> > Hi Arnd:
> > I have got a new idea about the problem:
> > In include/linux/time64.h
> > #define NSEC_PER_SEC10L
> > I think we should change it to
> > #define NSEC_PER_SEC10LL
> >
> > My reason is :
> > 1.  when it is used in a multiplication, it will easily get overflow.
> > 2.  when it don't get overflow, the change has no side affect.
> 
> That's not a good idea. NSEC_PER_SEC is used in lots of places with 32bit
> storage. I really don't want to mop up all the fallout.

Yes, agree, a lot of places it has been used as 32bit value. Beyond that, I 
think 
it is more reasonable to use 64bit. 

> 
> Thanks,
> 
>   tglx
> 

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

Hi Arnd:
I have got a new idea about the problem: 
In include/linux/time64.h
#define NSEC_PER_SEC10L 
I think we should change it to
#define NSEC_PER_SEC10LL 

My reason is :
1.  when it is used in a multiplication, it will easily get overflow.
2.  when it don't get overflow, the change has no side affect.

Thanks.
Zengtao

> -Original Message-
> From: Zengtao (B)
> Sent: Saturday, January 30, 2016 10:31 AM
> To: 'Arnd Bergmann'
> Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> Subject: RE: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> > -Original Message-
> > From: Arnd Bergmann [mailto:a...@arndb.de]
> > Sent: Friday, January 29, 2016 4:46 PM
> > To: Zengtao (B)
> > Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> >
> > On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > > > -Original Message-
> > > > From: Arnd Bergmann [mailto:a...@arndb.de]
> > > > Sent: Thursday, January 28, 2016 7:52 PM
> > > > To: Thomas Gleixner
> > > > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > > > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> > > >
> > > > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > > > Cc'ing Arnd
> > > > >
> > > > > On Thu, 28 Jan 2016, zengtao wrote:
> > > > >
> > > > > > The structure:
> > > > > > struct timeval {
> > > > > >   __kernel_time_t tv_sec; /* seconds */
> > > > > >   __kernel_suseconds_ttv_usec;/* microseconds
> > */
> > > > > > };
> > > > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > > > when it is 32bit platform, so force u64 conversion here.
> > > > > >
> > > > > > Signed-off-by: zengtao 
> > > > >
> > > > > Reviewed-by: Thomas Gleixner 
> > > >
> > > > This seems to miss timespec_to_cputime(), which has the same problem,
> > > > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> > > Yes, I have checked the code just now, the timespec_to_cputime() has the
> > > same problem.I found the origin issue through setitimer().And I think the
> > > timespec_to_cputime() only affects timer_settime(),by which means it
> > affects
> > > nanosleep?
> >
> > Reading that code again, I think it does not affect sys_nanosleep, but
> > it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
> > along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
> >
> Got it, I will fix the timespec_to_cputime and resend the patch later.
> > Arnd

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

Hi Arnd:
I have got a new idea about the problem: 
In include/linux/time64.h
#define NSEC_PER_SEC10L 
I think we should change it to
#define NSEC_PER_SEC10LL 

My reason is :
1.  when it is used in a multiplication, it will easily get overflow.
2.  when it don't get overflow, the change has no side affect.

Thanks.
Zengtao

> -Original Message-
> From: Zengtao (B)
> Sent: Saturday, January 30, 2016 10:31 AM
> To: 'Arnd Bergmann'
> Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> Subject: RE: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> > -Original Message-
> > From: Arnd Bergmann [mailto:a...@arndb.de]
> > Sent: Friday, January 29, 2016 4:46 PM
> > To: Zengtao (B)
> > Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> >
> > On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > > > -Original Message-
> > > > From: Arnd Bergmann [mailto:a...@arndb.de]
> > > > Sent: Thursday, January 28, 2016 7:52 PM
> > > > To: Thomas Gleixner
> > > > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > > > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> > > >
> > > > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > > > Cc'ing Arnd
> > > > >
> > > > > On Thu, 28 Jan 2016, zengtao wrote:
> > > > >
> > > > > > The structure:
> > > > > > struct timeval {
> > > > > >   __kernel_time_t tv_sec; /* seconds */
> > > > > >   __kernel_suseconds_ttv_usec;/* microseconds
> > */
> > > > > > };
> > > > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > > > when it is 32bit platform, so force u64 conversion here.
> > > > > >
> > > > > > Signed-off-by: zengtao <prime.z...@huawei.com>
> > > > >
> > > > > Reviewed-by: Thomas Gleixner <t...@linutronix.de>
> > > >
> > > > This seems to miss timespec_to_cputime(), which has the same problem,
> > > > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> > > Yes, I have checked the code just now, the timespec_to_cputime() has the
> > > same problem.I found the origin issue through setitimer().And I think the
> > > timespec_to_cputime() only affects timer_settime(),by which means it
> > affects
> > > nanosleep?
> >
> > Reading that code again, I think it does not affect sys_nanosleep, but
> > it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
> > along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
> >
> Got it, I will fix the timespec_to_cputime and resend the patch later.
> > Arnd

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Friday, January 29, 2016 4:46 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > > -Original Message-
> > > From: Arnd Bergmann [mailto:a...@arndb.de]
> > > Sent: Thursday, January 28, 2016 7:52 PM
> > > To: Thomas Gleixner
> > > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> > >
> > > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > > Cc'ing Arnd
> > > >
> > > > On Thu, 28 Jan 2016, zengtao wrote:
> > > >
> > > > > The structure:
> > > > > struct timeval {
> > > > >   __kernel_time_t tv_sec; /* seconds */
> > > > >   __kernel_suseconds_ttv_usec;/* microseconds
> */
> > > > > };
> > > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > > when it is 32bit platform, so force u64 conversion here.
> > > > >
> > > > > Signed-off-by: zengtao 
> > > >
> > > > Reviewed-by: Thomas Gleixner 
> > >
> > > This seems to miss timespec_to_cputime(), which has the same problem,
> > > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> > Yes, I have checked the code just now, the timespec_to_cputime() has the
> > same problem.I found the origin issue through setitimer().And I think the
> > timespec_to_cputime() only affects timer_settime(),by which means it
> affects
> > nanosleep?
> 
> Reading that code again, I think it does not affect sys_nanosleep, but
> it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
> along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
> 
Got it, I will fix the timespec_to_cputime and resend the patch later.
>   Arnd

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Friday, January 29, 2016 4:46 PM
> To: Zengtao (B)
> Cc: Thomas Gleixner; LKML; Frederic Weisbecker
> Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> On Friday 29 January 2016 03:12:37 Zengtao wrote:
> > > -Original Message-
> > > From: Arnd Bergmann [mailto:a...@arndb.de]
> > > Sent: Thursday, January 28, 2016 7:52 PM
> > > To: Thomas Gleixner
> > > Cc: Zengtao (B); LKML; Frederic Weisbecker
> > > Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> > >
> > > On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > > > Cc'ing Arnd
> > > >
> > > > On Thu, 28 Jan 2016, zengtao wrote:
> > > >
> > > > > The structure:
> > > > > struct timeval {
> > > > >   __kernel_time_t tv_sec; /* seconds */
> > > > >   __kernel_suseconds_ttv_usec;/* microseconds
> */
> > > > > };
> > > > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > > > when it is 32bit platform, so force u64 conversion here.
> > > > >
> > > > > Signed-off-by: zengtao <prime.z...@huawei.com>
> > > >
> > > > Reviewed-by: Thomas Gleixner <t...@linutronix.de>
> > >
> > > This seems to miss timespec_to_cputime(), which has the same problem,
> > > so only setitimer() is fixed, but not nanosleep() or timer_settime().
> > Yes, I have checked the code just now, the timespec_to_cputime() has the
> > same problem.I found the origin issue through setitimer().And I think the
> > timespec_to_cputime() only affects timer_settime(),by which means it
> affects
> > nanosleep?
> 
> Reading that code again, I think it does not affect sys_nanosleep, but
> it does affect sys_clock_nanosleep(CLOCK_PROCESS_CPUTIME_ID, ...)
> along with timer_create/timer_settime with CLOCK_PROCESS_CPUTIME_ID.
> 
Got it, I will fix the timespec_to_cputime and resend the patch later.
>   Arnd

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Thursday, January 28, 2016 7:52 PM
> To: Thomas Gleixner
> Cc: Zengtao (B); LKML; Frederic Weisbecker
> Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > Cc'ing Arnd
> >
> > On Thu, 28 Jan 2016, zengtao wrote:
> >
> > > The structure:
> > > struct timeval {
> > >   __kernel_time_t tv_sec; /* seconds */
> > >   __kernel_suseconds_ttv_usec;/* microseconds */
> > > };
> > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > when it is 32bit platform, so force u64 conversion here.
> > >
> > > Signed-off-by: zengtao 
> >
> > Reviewed-by: Thomas Gleixner 
> 
> This seems to miss timespec_to_cputime(), which has the same problem,
> so only setitimer() is fixed, but not nanosleep() or timer_settime().
Yes, I have checked the code just now, the timespec_to_cputime() has the 
same problem.I found the origin issue through setitimer().And I think the 
timespec_to_cputime() only affects timer_settime(),by which means it affects 
nanosleep? 

> 
> There should probably be some explanation in which cases this happens,
> my reading is that can only occur on MIPS32 and ARM32 with "Full dynticks
> CPU time accounting" enabled, which is required for CONFIG_NO_HZ_FULL,
> so we need this backported to any kernel that includes
> 31c1fc818715 ("ARM: Kconfig: allow full nohz CPU accounting"), i.e.
> v3.13 or higher, correct?
Yes.
> 
>   Arnd
> 
> > >  include/asm-generic/cputime_nsecs.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/asm-generic/cputime_nsecs.h
> b/include/asm-generic/cputime_nsecs.h
> > > index 0419485..e2f7ff9 100644
> > > --- a/include/asm-generic/cputime_nsecs.h
> > > +++ b/include/asm-generic/cputime_nsecs.h
> > > @@ -91,7 +91,8 @@ static inline void cputime_to_timespec(const
> cputime_t ct, struct timespec *val)
> > >   */
> > >  static inline cputime_t timeval_to_cputime(const struct timeval *val)
> > >  {
> > > - u64 ret = val->tv_sec * NSEC_PER_SEC + val->tv_usec *
> NSEC_PER_USEC;
> > > + u64 ret = (u64)val->tv_sec * NSEC_PER_SEC +
> > > + val->tv_usec * NSEC_PER_USEC;
> > >   return (__force cputime_t) ret;
> > >  }
> > >  static inline void cputime_to_timeval(const cputime_t ct, struct timeval
> *val)
> > > --
> > > 1.9.1
> > >
> > >

RE: [PATCH] cputime: Fix timeval-->cputime conversion

[Zengtao (B)]

> -Original Message-
> From: Arnd Bergmann [mailto:a...@arndb.de]
> Sent: Thursday, January 28, 2016 7:52 PM
> To: Thomas Gleixner
> Cc: Zengtao (B); LKML; Frederic Weisbecker
> Subject: Re: [PATCH] cputime: Fix timeval-->cputime conversion
> 
> On Thursday 28 January 2016 09:22:04 Thomas Gleixner wrote:
> > Cc'ing Arnd
> >
> > On Thu, 28 Jan 2016, zengtao wrote:
> >
> > > The structure:
> > > struct timeval {
> > >   __kernel_time_t tv_sec; /* seconds */
> > >   __kernel_suseconds_ttv_usec;/* microseconds */
> > > };
> > > both __kernel_time_t and __kernel_suseconds_t are short than u64
> > > when it is 32bit platform, so force u64 conversion here.
> > >
> > > Signed-off-by: zengtao <prime.z...@huawei.com>
> >
> > Reviewed-by: Thomas Gleixner <t...@linutronix.de>
> 
> This seems to miss timespec_to_cputime(), which has the same problem,
> so only setitimer() is fixed, but not nanosleep() or timer_settime().
Yes, I have checked the code just now, the timespec_to_cputime() has the 
same problem.I found the origin issue through setitimer().And I think the 
timespec_to_cputime() only affects timer_settime(),by which means it affects 
nanosleep? 

> 
> There should probably be some explanation in which cases this happens,
> my reading is that can only occur on MIPS32 and ARM32 with "Full dynticks
> CPU time accounting" enabled, which is required for CONFIG_NO_HZ_FULL,
> so we need this backported to any kernel that includes
> 31c1fc818715 ("ARM: Kconfig: allow full nohz CPU accounting"), i.e.
> v3.13 or higher, correct?
Yes.
> 
>   Arnd
> 
> > >  include/asm-generic/cputime_nsecs.h | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/asm-generic/cputime_nsecs.h
> b/include/asm-generic/cputime_nsecs.h
> > > index 0419485..e2f7ff9 100644
> > > --- a/include/asm-generic/cputime_nsecs.h
> > > +++ b/include/asm-generic/cputime_nsecs.h
> > > @@ -91,7 +91,8 @@ static inline void cputime_to_timespec(const
> cputime_t ct, struct timespec *val)
> > >   */
> > >  static inline cputime_t timeval_to_cputime(const struct timeval *val)
> > >  {
> > > - u64 ret = val->tv_sec * NSEC_PER_SEC + val->tv_usec *
> NSEC_PER_USEC;
> > > + u64 ret = (u64)val->tv_sec * NSEC_PER_SEC +
> > > + val->tv_usec * NSEC_PER_USEC;
> > >   return (__force cputime_t) ret;
> > >  }
> > >  static inline void cputime_to_timeval(const cputime_t ct, struct timeval
> *val)
> > > --
> > > 1.9.1
> > >
> > >