陈华才 江苏中科梦兰电子科技有限公司/自主安全事业部/软件部 江苏常熟虞山镇梦兰村 ------------------ Original ------------------ From: "Thomas Gleixner"<tglx@xxxxxxxxxxxxx>; Date: Wed, Jan 16, 2019 05:26 PM To: "陈华才"<chenhc@xxxxxxxxxx>; Cc: "linux-kernel"<linux-kernel@xxxxxxxxxxxxxxx>; "Fuxin Zhang"<zhangfx@xxxxxxxxxx>; "wuzhangjin"<wuzhangjin@xxxxxxxxx>; "stable"<stable@xxxxxxxxxxxxxxx>; "Christoph Hellwig"<hch@xxxxxx>; "Michael Hernandez"<michael.hernandez@xxxxxxxxxx>; Subject: Re: [PATCH] genirq/affinity: Assign default affinity to pre/post vectors Chen, On Wed, 16 Jan 2019, 陈华才 wrote: > please do not top-post and use line breaks around 78 char. > I'm not removing all return NULL of irq_create_affinity_masks(), so the ...... > > Moved content to the place where it belongs so the context is preserved. > ------------------ Original ------------------ > From: "Thomas Gleixner"<tglx@xxxxxxxxxxxxx>; > Date: Wed, Jan 16, 2019 03:10 AM > To: "Huacai Chen"<chenhc@xxxxxxxxxx>; > Cc: "linux-kernel"<linux-kernel@xxxxxxxxxxxxxxx>; "Fuxin Zhang"<zhangfx@xxxxxxxxxx>; "wuzhangjin"<wuzhangjin@xxxxxxxxx>; "stable"<stable@xxxxxxxxxxxxxxx>; "Christoph Hellwig"<hch@xxxxxx>; "Michael Hernandez"<michael.hernandez@xxxxxxxxxx>; > Subject: Re: [PATCH] genirq/affinity: Assign default affinity to pre/post vectors > And please configure your e-mail client to NOT copy the full headers into > the reply. > > > On Mon, 31 Dec 2018, Huacai Chen wrote: > > > > > > > Generally, irq_create_affinity_masks() assign default affinity to pre/ > > > > post vectors correctly. However, it ignore the case that there are only > > > > pre/post vectors (when nvecs == affd->pre_vectors + affd->post_vectors) > > > > and return NULL. This case usually happens when nvecs = 1 (e.g. in nvme > > > > driver when MSI-X is unavailable and fallback to MSI) and will trigger > > > > the warning in pci_irq_get_affinity(). This patch fix the corner case. > > > > > > Errm. This is just wrong. When this function returns NULL, then it has > > > failed and the caller or any subsequent code is not supposed to use the > > > result. > > > > > > The function can return NULL for other reasons, e.g. when the memory > > > allocation failed. How are you going to duct tape that one? > > > > I'm not removing all return NULL of irq_create_affinity_masks(), so the > > memory allocation failure still return NULL. I just handle the case that > > there are not enough irq vectors. E.g. in nvme driver, the caller may call > > irq_create_affinity_masks() with nvecs=1,pre_vectors=1,post_vectors=0. In > > this case, the only one vector's default affinity assigning is skipped. > > I did not say that you removed all NULL returns. I said that this function > can return NULL for other reasons and then the same situation will happen. > > If the masks pointer returned is NULL then the calling code or any > subsequent usage needs to handle it properly. Yes, I understand that this > change makes the warning go away for that particular case, but that's not > making it any more correct. Hi, Thomas, I don't think "nvecs == affd->pre_vectors + affd->post_vectors" is an ERROR, so it should be different with "return NULL for other reasons" to the caller. If the caller fallback from MSI-X to MSI, it is probably "nvecs=1,pre_vectors=1, post_vectors=0". The caller can work perfectly, if pre/post vectors are filled with the default affinity. > Thanks, > tglx
