Re: [PATCH v4 5/9] staging/rdma/hfi1: Add function stubs for TID caching
On Fri, Nov 06, 2015 at 05:02:35PM -0800, Greg KH wrote: > On Fri, Oct 30, 2015 at 06:58:44PM -0400, ira.we...@intel.com wrote: > > From: Mitko Haralanov > > > > Add mmu notify helper functions and TID caching function stubs in > > preparation > > for the TID caching implementation. > > > > TID caching makes use of the MMU notifier to allow the driver to respond to > > the > > user freeing memory which is allocated to the HFI. > > > > This patch implements the basic MMU notifier functions to insert, find and > > remove buffer pages from memory based on the mmu_notifier being invoked. > > > > In addition it places stubs in place for the main entry points by follow on > > code. > > > > Follow up patches will complete the implementation of the interaction with > > user > > space and makes use of these functions. > > This patch adds warnings to the build, which isn't ok, so I've stopped > here in this series and just applied the first 4. Please fix up and > resend the remaining ones. Ok, patch 7-9 of this series do not depend on this patch nor number 6. I will resend those 3 while I figure out what to do about these 2: staging/rdma/hfi1: Add function stubs for TID caching staging/rdma/hfi1: Implement Expected Receive TID caching Frankly this was an attempt to reduce the size of "Implement Expected Receive TID caching". I obviously did something wrong. I really don't know that I can split these up any more without causing issues with bisecting the code. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 6/9] staging/rdma/hfi1: Implement Expected Receive TID caching
On Fri, Nov 06, 2015 at 05:03:28PM -0800, Greg KH wrote: > On Fri, Oct 30, 2015 at 06:58:45PM -0400, ira.we...@intel.com wrote: > > From: Mitko Haralanov > > > > Expected receives work by user-space libraries (PSM) calling into the driver > > with information about the user's receive buffer and have the driver DMA-map > > that buffer and program the HFI to receive data directly into it. > > > > This is an expensive operation as it requires the driver to pin the pages > > which > > the user's buffer maps to, DMA-map them, and then program the HFI. > > > > When the receive is complete, user-space libraries have to call into the > > driver > > again so the buffer is removed from the HFI, un-mapped, and the pages > > unpinned. > > > > All of these operations are expensive, considering that a lot of > > applications > > (especially micro-benchmarks) use the same buffer over and over. > > > > In order to get better performance for user-space applications, it is highly > > beneficial that they don't continuously call into the driver to register and > > unregister the same buffer. Rather, they can register the buffer and cache > > it > > for future work. The buffer can be unregistered when it is freed by the > > user. > > > > This change implements such buffer caching by making use of the kernel's MMU > > notifier API. User-space libraries call into the driver only when the need > > to > > register a new buffer. > > > > Once a buffer is registered, it stays programmed into the HFI until the > > kernel > > notifies the driver that the buffer has been freed by the user. At that > > time, > > the user-space library is notified and it can do the necessary work to > > remove > > the buffer from its cache. > > > > Buffers which have been invalidated by the kernel are not automatically > > removed > > from the HFI and do not have their pages unpinned. Buffers are only > > completely > > removed when the user-space libraries call into the driver to free them. > > This > > is done to ensure that any ongoing transfers into that buffer are complete. > > This is important when a buffer is not completely freed but rather it is > > shrunk. The user-space library could still have uncompleted transfers into > > the > > remaining buffer. > > > > With this feature, it is important that systems are setup with reasonable > > limits for the amount of lockable memory. Keeping the limit at "unlimited" > > (as > > we've done up to this point), may result in jobs being killed by the > > kernel's > > OOM due to them taking up excessive amounts of memory. > > > > Reviewed-by: Arthur Kepner > > Reviewed-by: Dennis Dalessandro > > Signed-off-by: Mitko Haralanov > > Signed-off-by: Ira Weiny > > > > --- > > Changes from V3: > > Reworked based on the removal of the file pointer macros > > Split out some prep patches and code clean up > > > > Changes from V2: > > Fix random Kconfig 0-day build error > > Fix leak of random memory to user space caught by Dan Carpenter > > Separate out pointer bug fix into a previous patch > > Change error checks in case statement per Dan's comments > > > > drivers/staging/rdma/hfi1/file_ops.c | 469 ++--- > > drivers/staging/rdma/hfi1/hfi.h | 43 +- > > drivers/staging/rdma/hfi1/init.c | 5 +- > > drivers/staging/rdma/hfi1/trace.h| 132 +++-- > > drivers/staging/rdma/hfi1/user_exp_rcv.c | 874 > > ++- > > drivers/staging/rdma/hfi1/user_pages.c | 110 +--- > > drivers/staging/rdma/hfi1/user_sdma.c| 13 + > > include/uapi/rdma/hfi/hfi1_user.h| 14 +- > > 8 files changed, 1069 insertions(+), 591 deletions(-) > > This is still a really big patch, any chance you can break it up into > smaller, reviewable parts? I see you add different operations, perhaps > break it up into one patch per logical thing? > I understand, however, as you have seen with my attempt to break this up there are issues if we do so. I need to clean up the previous patch which was an attempt to split this one up. But at this point I would really like to preserve the functionality we have here. Breaking this up beyond this point is going to be difficult to do and really will not allow for bisecting the code across this feature being in vs out. Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] staging/rdma/hfi1: set Gen3 half-swing for integrated devices
On Tue, Nov 10, 2015 at 12:59:29PM +0300, Dan Carpenter wrote:
> Gar... No. Please please get rid of the PC() macro. It makes the code
> impossible to understand because instead of hitting CTRL-[ you have
> decode it and then manually type out
>
> :cs find g CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_OVERWRITE_ENABLE_SHIFT
>
> which is the length of a typical college essay. I meant just put a
> comment like this:
>
> /*
> * In the hardware spec these are prefixed with:
> * CCE_PCIE_CTRL_...
> * But it is too long to use in code.
> */
> #define XMT_MARGIN_GEN1_GEN2_OVERWRITE_ENABLE_MASK 0x1ull
>
> Or probably even better:
>
> #define CCE_PCIE_CTRL (CCE + 0x00C0)
> #define LANE_BUNDLE_MASK 0x3ull /* CCE_PCIE_CTRL_PCIE_LANE_BUNDLE_MASK */
> #define LANE_BUNDLE_SHIFT 0 /* CCE_PCIE_CTRL_PCIE_LANE_BUNDLE_SHIFT
> */
> #define LANE_DELAY_MASK 0xFull /*
> CCE_PCIE_CTRL_PCIE_LANE_DELAY_MASK */
> #define LANE_DELAY_SHIFT 2 /* CCE_PCIE_CTRL_PCIE_LANE_DELAY_SHIFT */
> #define MARGIN_OVERWRITE_SHIFT8 /*
> CCE_PCIE_CTRL_XMT_MARGIN_OVERWRITE_ENABLE_SHIFT */
> #define MARGIN_SHIFT 9 /* CCE_PCIE_CTRL_XMT_MARGIN_SHIFT */
> #define MARGIN_G1G2_OVERWRITE_MASK 0x1ull /*
> CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_OVERWRITE_ENABLE_MASK */
> #define MARGIN_G1G2_OVERWRITE_SHIFT 12/*
> CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_OVERWRITE_ENABLE_SHIFT */
> #define MARGIN_G1G2_MASK 0x7ull /*
> CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_MASK */
> #define MARGIN_G1G2_SHIFT 13 /*
> CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_SHIFT */
>
> Those lines go over the 80 character limit but it's fine.
My apologies for not understanding what you meant. I took your meaning to be
that we had to honor the checkpatch checks so while the PC macro was
undesirable it was ok if I just made some comments...
FWIW I don't like the PC macro either. But we have a tool which is generating
these names to be identical to the hardware spec. And we really want to
preserve those as a reference back to the spec. Creating additional names which
are in the code is a bit cumbersome but what if we do something like this:
...
#define CCE_PCIE_CTRL (CCE + 0x00C0)
#define CCE_PCIE_CTRL_PCIE_LANE_BUNDLE_MASK 0x3ull
#define CCE_PCIE_CTRL_PCIE_LANE_BUNDLE_SHIFT 0
...
#define LANE_BUNDLE_MASKCCE_PCIE_CTRL_PCIE_LANE_BUNDLE_MASK
#define LANE_BUNDLE_SHIFT CCE_PCIE_CTRL_PCIE_LANE_BUNDLE_SHIFT
...
?
An alternative would be to define some helper functions such as:
static inline u64 extract_xmt_margin_g1g2(u64 reg)
{
return (reg >> CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_SHIFT)
& CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_MASK;
}
...
...
xmt_margin = extract_xmt_margin_g1g2(pcie_ctrl);
...
I prefer the second option as it preserves the register names right in the
code. So you can reference the hardware spec without looking anything up in a
header file.
I again apologize for misunderstanding your previous meaning.
Ira
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [BUG] IB/hfi1: might sleep under spinlock in hfi1_ioctl()
On Fri, Oct 30, 2015 at 07:58:18PM -0400, ira. weiny wrote: > On Sat, Oct 31, 2015 at 12:32:29AM +0300, Alexey Khoroshilov wrote: > > Hello, > > > > hfi1_ioctl() contains many calls to might sleep functions with > > dd->hfi1_snoop.snoop_lock spinlock held (for example, access_ok, > > copy_from_user, kzalloc(GFP_KERNEL), etc.). > > > > Should dd->hfi1_snoop.snoop_lock be acquired just before updating state? > > I believe you are correct. > > I am currently in the process of pushing fixes to the staging tree. > > We have a patch which fixes this queued up but it depends on at least one > other > patch in my queue. > > I will do my best to get this submitted soon. I have just posted a series which addresses this problem as well as doing general clean up on hfi1_ioctl. The specific fix is contained in this patch. [PATCH 7/8] staging/rdma/hfi1: Reduce snoop locking scope in IOCTL handler. Thanks for the report, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 5/9] staging/rdma/hfi1: Add function stubs for TID caching
> > > > Ok, patch 7-9 of this series do not depend on this patch nor number 6. I > > will > > resend those 3 while I figure out what to do about these 2: > > > > staging/rdma/hfi1: Add function stubs for TID caching > > staging/rdma/hfi1: Implement Expected Receive TID caching > > > > Frankly this was an attempt to reduce the size of "Implement Expected > > Receive > > TID caching". I obviously did something wrong. > > > > I really don't know that I can split these up any more without causing > > issues > > with bisecting the code. > > I strongly doubt that you created this new feature all "at once", it > took a set of steps to get to your final destination. So show that > work, like your math professor told you... > The original author and I have been going through the code to see what we can do. We have identified a couple of other pieces which can be split. One question. Is it ok to have functionality which is added which is unused in a preliminary patch? I believe this is ok as long as the code compiles but I just wanted to make sure. While there are different operations added in this patch it is broken to not use them as a set. So we need to have a series which implement the pieces with a final patch which exposes the set of operations. Is this acceptable? Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 4/8] staging/rdma/hfi1: remove unneeded goto done
On Wed, Nov 11, 2015 at 12:01:08PM +0300, Dan Carpenter wrote: > On Wed, Nov 11, 2015 at 12:43:05AM -0500, ira.we...@intel.com wrote: > > From: Ira Weiny > > > > This goto done is followed by an if (ret) break in the outer switch clause. > > It > > is unnecessary. > > > > Signed-off-by: Dennis Dalessandro > > Signed-off-by: Ira Weiny > > --- > > drivers/staging/rdma/hfi1/diag.c | 3 +-- > > 1 file changed, 1 insertion(+), 2 deletions(-) > > > > diff --git a/drivers/staging/rdma/hfi1/diag.c > > b/drivers/staging/rdma/hfi1/diag.c > > index 2bb857b2a103..556a47591989 100644 > > --- a/drivers/staging/rdma/hfi1/diag.c > > +++ b/drivers/staging/rdma/hfi1/diag.c > > @@ -1053,7 +1053,6 @@ static long hfi1_ioctl(struct file *fp, unsigned int > > cmd, unsigned long arg) > > break; > > default: > > ret = -EINVAL; > > - goto done; > > } > > ret = set_link_state(ppd, dev_state); > > break; > > Wut? No it's not. Sorry, you are correct at this point in the series. I got over zealous with my splitting of this patch. This needs to be squashed into staging/rdma/hfi1: Return immediately on error Which returns here. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Panic in drm_calc_timestamping_constants in staging-next
With the latest staging-testing and staging-next[*] I am getting the following panic. [*] git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git [ 11.232549] BUG: unable to handle kernel NULL pointer dereference at 00b0 [ 11.232568] IP: [] drm_calc_timestamping_constants+0x86/0x130 [drm] [ 11.232571] PGD 0 [ 11.232574] Oops: 0002 [#1] SMP [ 11.232595] Modules linked in: ib_qib mgag200(+) drm_kms_helper isci syscopyarea sysfillrect sysimgblt fb_sys_fops ib_mad ttm libsas mlx4_core(+) ib_core igb drm ahci scsi_transport_sas libahci ptp libata firewire_ohci ib_addr pps_core firewire_core dca i2c_algo_bit i2c_core crc_itu_t [ 11.232600] CPU: 13 PID: 497 Comm: systemd-udevd Not tainted 4.3.0+ #1 [ 11.232601] Hardware name: Intel Corporation W2600CR ../W2600CR, BIOS SE5C600.86B.01.08.0003.022620131521 02/26/2013 [ 11.232603] task: 8800343abfc0 ti: 8804244a8000 task.ti: 8804244a8000 [ 11.232618] RIP: 0010:[] [] drm_calc_timestamping_constants+0x86/0x130 [drm] [ 11.232620] RSP: 0018:8804244ab118 EFLAGS: 00010246 [ 11.232621] RAX: 00fe4c00 RBX: 880424b10160 RCX: 0540 [ 11.232623] RDX: RSI: fde8 RDI: 880424b1 [ 11.232624] RBP: 8804244ab148 R08: 8804244a8000 R09: 00029d828339 [ 11.232626] R10: 50c4 R11: R12: 00fe4c00 [ 11.232627] R13: 880424b1 R14: R15: fde8 [ 11.232629] FS: 7fecf960d880() GS:88082d94() knlGS: [ 11.232631] CS: 0010 DS: ES: CR0: 80050033 [ 11.232632] CR2: 00b0 CR3: 000424493000 CR4: 000406e0 [ 11.232634] Stack: [ 11.232637] 8804244ab148 880424b1 88042a86bb40 88042a86b800 [ 11.232639] 88042a86bb48 88042a86bb40 8804244ab378 a030c7e7 [ 11.232642] 880424b10090 880424b10160 [ 11.232642] Call Trace: [ 11.232655] [] drm_crtc_helper_set_mode+0x3d7/0x4b0 [drm_kms_helper] [ 11.232665] [] drm_crtc_helper_set_config+0x8d4/0xb10 [drm_kms_helper] [ 11.232683] [] drm_mode_set_config_internal+0x64/0x100 [drm] [ 11.232694] [] drm_fb_helper_pan_display+0xa2/0x280 [drm_kms_helper] [ 11.232703] [] fb_pan_display+0xbb/0x170 [ 11.232708] [] bit_update_start+0x20/0x50 [ 11.232712] [] fbcon_switch+0x39b/0x590 [ 11.232721] [] redraw_screen+0x1a0/0x240 [ 11.232725] [] vc_do_resize+0x4d8/0x500 [ 11.232729] [] vc_resize+0x1f/0x30 [ 11.232732] [] fbcon_init+0x342/0x530 [ 11.232737] [] visual_init+0xca/0x130 [ 11.232741] [] do_bind_con_driver+0x146/0x310 [ 11.232746] [] do_take_over_console+0x141/0x1b0 [ 11.232750] [] do_fbcon_takeover+0x57/0xb0 [ 11.232754] [] fbcon_event_notify+0x60b/0x750 [ 11.232760] [] notifier_call_chain+0x49/0x70 [ 11.232764] [] __blocking_notifier_call_chain+0x4d/0x70 [ 11.232768] [] blocking_notifier_call_chain+0x16/0x20 [ 11.232772] [] fb_notifier_call_chain+0x1b/0x20 [ 11.232775] [] register_framebuffer+0x1f1/0x330 [ 11.232784] [] drm_fb_helper_initial_config+0x27a/0x3d0 [drm_kms_helper] [ 11.232792] [] mgag200_fbdev_init+0xdd/0xf0 [mgag200] [ 11.232798] [] mgag200_modeset_init+0x176/0x1e0 [mgag200] [ 11.232804] [] mgag200_driver_load+0x3f9/0x580 [mgag200] [ 11.232819] [] drm_dev_register+0xa7/0xb0 [drm] [ 11.232834] [] drm_get_pci_dev+0x8f/0x1e0 [drm] [ 11.232840] [] mga_pci_probe+0x9b/0xc0 [mgag200] [ 11.232848] [] local_pci_probe+0x45/0xa0 [ 11.232853] [] pci_device_probe+0xfc/0x140 [ 11.232858] [] driver_probe_device+0x21b/0x460 [ 11.232861] [] __driver_attach+0x85/0x90 [ 11.232864] [] ? driver_probe_device+0x460/0x460 [ 11.232868] [] bus_for_each_dev+0x6c/0xc0 [ 11.232871] [] driver_attach+0x1e/0x20 [ 11.232873] [] bus_add_driver+0x1d0/0x290 [ 11.232876] [] driver_register+0x60/0xe0 [ 11.232880] [] __pci_register_driver+0x4c/0x50 [ 11.232894] [] drm_pci_init+0xe0/0x110 [drm] [ 11.232897] [] ? 0xa0348000 [ 11.232902] [] mgag200_init+0x32/0x1000 [mgag200] [ 11.232907] [] do_one_initcall+0xcd/0x1f0 [ 11.232911] [] ? __vunmap+0xa6/0xf0 [ 11.232918] [] ? kmem_cache_alloc_trace+0x17b/0x1e0 [ 11.232921] [] ? do_init_module+0x27/0x1e8 [ 11.232924] [] do_init_module+0x60/0x1e8 [ 11.232930] [] load_module+0x12b3/0x1980 [ 11.232933] [] ? __symbol_put+0x60/0x60 [ 11.232938] [] ? copy_module_from_fd.isra.51+0x110/0x160 [ 11.232943] [] SyS_finit_module+0x9f/0xd0 [ 11.232949] [] entry_SYSCALL_64_fastpath+0x12/0x71 [ 11.232976] Code: f6 31 d2 41 89 c2 8b 83 b4 00 00 00 0f af c1 48 98 48 69 c0 40 42 0f 00 48 f7 f6 f6 43 74 10 41 89 c4 75 26 f6 05 fa 6f 03 00 01 <45> 89 96 b0 00 00 00 45 89 a6 ac 00 00 00 75 35 48 83 c4 08 5b [ 11.232990] RIP [] drm_calc_timestamping_constants+0x86/0x130 [drm] [ 11.232991] RSP [ 11.232992] CR2: 00
Re: Panic in drm_calc_timestamping_constants in staging-next
On Sun, Nov 15, 2015 at 08:09:18PM -0800, Greg KH wrote: > On Sun, Nov 15, 2015 at 01:17:00PM -0500, ira.weiny wrote: > > With the latest staging-testing and staging-next[*] I am getting the > > following panic. > > > > [*] git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git > > So 4.4-rc1 also has this issue? Not sure about 4.4-rc1 I thought I tried master last week but I think I may have been pulling the wrong tree from kernel.org. Let me try again. > > Can you use 'git bisect' to track it down? I really doubt it's due to > anything in my tree, as my trees are empty now... I used bisect to track it down to the commit I referenced. But as I said the behavior of the bug changed a bit (hang vs panic). So I'm not sure if that commit caused the panic directly. I'm still looking into it but I thought perhaps the authors of those patches in question may have some insight. This did happen on your trees but I see you have some additional patches this morning. I'll try 4.4-rc1 Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Panic in drm_calc_timestamping_constants in staging-next
On Mon, Nov 16, 2015 at 10:00:04AM -0500, ira. weiny wrote: > On Sun, Nov 15, 2015 at 08:09:18PM -0800, Greg KH wrote: > > On Sun, Nov 15, 2015 at 01:17:00PM -0500, ira.weiny wrote: > > > With the latest staging-testing and staging-next[*] I am getting the > > > following panic. > > > > > > [*] git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git > > > > So 4.4-rc1 also has this issue? > > Not sure about 4.4-rc1 I thought I tried master last week but I think I may > have been pulling the wrong tree from kernel.org. Let me try again. It does not happen 4.4-rc1... And I just tested with your updated staging-testing and it worked... I'm seeing different commit IDs for the HEAD of these trees. Current head of staging-testing 4811789503d1 staging: unisys: visorbus: visorbus_main.c: made checkpatch wa rning-free be1d6cb3e657 Staging: fwserial: Declare fwtty_port_put as static 3bf40d65dcbf staging/rdma/hfi1: use one-shot LCB write My previous head. 26b4a300d775 staging: unisys: visorbus: visorbus_main.c: made checkpatch warning -free ec1bacdc52b1 Staging: fwserial: Declare fwtty_port_put as static 2a3e9651000b staging/rdma/hfi1: use one-shot LCB write How would that happen? I did not rebase the tree... At least that I recall? I was fetching your tree all weekend to see if there was something different and nothing changed until this morning? :-/ I'll continue to work on your latest trees. Sorry for the noise... Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Panic in drm_calc_timestamping_constants in staging-next
On Mon, Nov 16, 2015 at 07:29:46PM +0300, Dan Carpenter wrote: > Ville Syrjälä already posted the answer but his email had no text just a > link inside 300+ lines of quoted output so you may have missed it. AFAICS I have not received anything from Ville. Thanks for the heads up, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote: > On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.we...@intel.com wrote: > > From: Jubin John > > > > Correct the checks on diag.c with the latest checkpatch > > > > Reviewed-by: Dennis Dalessandro > > Reviewed-by: Mike Marciniszyn > > Signed-off-by: Jubin John > > Signed-off-by: Ira Weiny > > --- > > Different types of changes in a single patch. Greg, Do you want me to resubmit the entire series for this? Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: HFI1 code duplication todo
On Fri, Nov 20, 2015 at 10:41:16AM -0500, Doug Ledford wrote: > On 11/19/2015 05:23 PM, Dennis Dalessandro wrote: > > On Thu, Nov 12, 2015 at 04:13:18PM -0500, Dennis Dalessandro wrote: > >> The major todo for the hfi1 driver in staging is getting rid of the > >> verbs code duplication between ipath, qib, and now hfi1. The ipath > >> driver has been deprecated and is going to be deleted soon. So that > >> leaves qib and hfi. To address this we have proposed rdmavt which will > >> be a common kmod that provides software RDMA verbs support. qib and > >> hfi1 will both use this as well as other forthcoming drivers such as > >> soft-roce. See this thread for some details: > >> http://www.spinics.net/lists/linux-rdma/msg29922.html. > >> > >> Since that initial RFC we have been accumulating patches in a GitHub > >> repo for an early look at the development. At some point soon we want > >> to actually start posting the patches to the mailing list. This is > >> where it gets tricky. The code basically not only adds a new driver > >> but it modifies two existing ones, heavily. To make it more murky one > >> driver is in staging the other is in the usual drivers/infiniband tree. > >> > >> The question is, how do we go about this logistically due to the 2 > >> drivers being in separate sub-trees? > >> > >> Greg, Doug, > >> As the maintainers of the two trees involved we'd kind of like to get > >> your thoughts on this. > > > > Hi Greg and Doug, > > > > Just wanted to ping you guys again in case my mail last week slipped > > through the cracks. We are at the point now where we have some patches > > we can start posting. Looking for some logistical guidance. > > Sorry, I've been out for a while now with the birth of my second child. > Things have settled down enough around here that I should be able to > get things going again (at least well enough to be more responsive to > email anyway). Congratulations! > > So, as to the hfi1/qib/rxe transport library. The qib driver is in the > rdma tree, and we aren't going to move it to staging just because it > depends on something in staging, so we need to start adding the library > in the core tree to avoid dependency issues. As for the hfi1 driver, > I'm of the opinion that putting it in staging because of the code > duplication issue was probably not the right thing to do. It's > benefited from the extra eyes on it to help make it a better driver, but > I think I'm ready to move it to the main RDMA tree and start working on > it from there where there won't be any cross tree dependency issues. > > To that end, I've opened my 4.4-rc branch and deleted the three > deprecated drivers from staging and moved hfi1 to the rdma tree. I've > sent an email to Linus to see if he's ok taking those changes, and if > so, I'll get them submitted and then open up my for-4.5 branch early to > be able to start taking for-next patches. I agree this is the correct way to go. However, we have patches which are either in staging-next, staging-testing, or in flight. How should we handle those? I propose. Greg can ignore the patches in flight. We have reworks on them anyway. Those which just went in to staging-testing can get dropped and we will resubmit them. For those which are in staging-next we can submit revert patches to avoid merge conflicts and resubmit those as well. This is the item I am not sure about? Or is there a better more standard way of handling this? Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: HFI1 code duplication todo
On Sat, Nov 21, 2015 at 01:25:23AM +0300, Dan Carpenter wrote: > On Fri, Nov 20, 2015 at 10:41:16AM -0500, Doug Ledford wrote: > > So, as to the hfi1/qib/rxe transport library. The qib driver is in the > > rdma tree, and we aren't going to move it to staging just because it > > depends on something in staging, so we need to start adding the library > > in the core tree to avoid dependency issues. As for the hfi1 driver, > > I'm of the opinion that putting it in staging because of the code > > duplication issue was probably not the right thing to do. It's > > benefited from the extra eyes on it to help make it a better driver, but > > I think I'm ready to move it to the main RDMA tree and start working on > > it from there where there won't be any cross tree dependency issues. > > You could leave it staging but manage it from the same git tree. It > avoids the cross tree dependencies. This is what we do for > drivers/staging/media/, all that stuff goes through the linux-media > tree. My concern is that we tried this before and it did not work. https://www.mail-archive.com/linux-rdma%40vger.kernel.org/msg27842.html https://www.mail-archive.com/linux-rdma%40vger.kernel.org/msg27858.html Shall we try it again? Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 3/3] staging/rdma/hfi1: Method to toggle "fast ECN" detection
On Thu, Nov 19, 2015 at 04:54:44PM -0800, Greg KH wrote: > On Mon, Nov 09, 2015 at 06:34:44PM -0500, ira.we...@intel.com wrote: > > From: Vennila Megavannan > > > > Add a module paramter to toggle prescan/Fast ECN Detection and remove the > > Kconfig option which used to control this. > > Ick, no, not a module parameter, that's horrid (hint, it isn't a > per-device option...) This is a good point. Previous to this patch we had a compile time option which would have affected all devices and I think we just continued that. I do like the idea of making this per port. I will respin the patch. However, I want to be clear on your hint. Are you saying that sysfs would be a better place to put such a flag? > > Why can't you do this dynamically? ECN is always on. The key is the reaction time of the individual port. Attempting to turn this on and off would affect both the reaction time and the processing time in a negative way. > Why would anyone ever want to make this "slow"? This is a tuning nob for over all fabric performance not individual node performance. ECN controls congestion spreading through the network as is explained in this paper. http://infocom2003.ieee-infocom.org/papers/28_01.PDF As is shown in figure 1 and 2 of that paper congestion at node Bc is affecting traffic at node Bv. The default reaction time of ECN is likely to be sufficient for most users based on our experience so far. However, should a particular network see system wide degradation, this option can be turned on to increase the reaction time at node Bc. However, node Bc is already overloaded so the trade off is likely acceptable. Unfortunately, it is hard to predict when a user may need this option as we don't have the resources to build extreme scale fabrics for testing. Nor do we know all users workloads or the fabric topologies those workloads may be running on. We developed the code which this option enables based on lab experiments. So we need this to be an option available to users. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 1/7] staging/rdma/hfi1: diag.c Correct code style issues
On Tue, Nov 17, 2015 at 02:27:46PM -0800, gre...@linuxfoundation.org wrote: > On Tue, Nov 17, 2015 at 03:30:24PM -0500, ira.weiny wrote: > > On Tue, Nov 17, 2015 at 10:17:26PM +0530, Sudip Mukherjee wrote: > > > On Mon, Nov 16, 2015 at 05:32:34PM -0500, ira.we...@intel.com wrote: > > > > From: Jubin John > > > > > > > > Correct the checks on diag.c with the latest checkpatch > > > > > > > > Reviewed-by: Dennis Dalessandro > > > > Reviewed-by: Mike Marciniszyn > > > > Signed-off-by: Jubin John > > > > Signed-off-by: Ira Weiny > > > > --- > > > > > > Different types of changes in a single patch. > > > > Greg, > > > > Do you want me to resubmit the entire series for this? > > Why wouldn't you? :) > I meant instead of just replacing this single patch with a new "pre" series. It is easier to just resend the entire series. I can do that once we settle whos tree these patches are going to land in. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 0/2] Fix 4.4 IB merge window regressions
On Mon, Nov 30, 2015 at 09:34:15AM -0500, Mike Marciniszyn wrote: > This two patch series fixes regressions for qib and hfi1 > introduced in the 4.4 merge window. > > These are critical for 4.4 and for rebasing the qib/hfi1 > refactoring. Doug, Greg, It should also be noted that without this change to uverbs_cmd the hfi1 driver is at least partially (ib_write_bw test) broken in staging-next. Greg, if Linus pulls these from Doug into rc4 will you pull them into staging-next? Ira > > --- > > Ira Weiny (1): > IB/qib: Fix qib_mr structure > > Mike Marciniszyn (1): > IB/core: correct issue with sge copyin corrupting wr > > > drivers/infiniband/core/uverbs_cmd.c | 15 ++- > drivers/infiniband/hw/qib/qib_verbs.h |2 +- > 2 files changed, 11 insertions(+), 6 deletions(-) > > -- > Mike > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[jubin.j...@intel.com: [PATCH] staging/rdma/hfi1: Fix error in hfi1 driver build]
Greg,
It looks like you missed this patch for 4.4-rc5
Could we get it included for rc6?
Thanks,
Ira
- Forwarded message from Jubin John -
From: Jubin John
To: ,
CC: , ,
, ,
Subject: [PATCH] staging/rdma/hfi1: Fix error in hfi1 driver build
Date: Fri, 20 Nov 2015 18:13:08 -0500
hfi1 driver build fails with the following error:
In function ‘handle_receive_interrupt’:
error: implicit declaration of function ‘skip_rcv_packet’
[-Werror=implicit-function-declaration]
last = skip_rcv_packet(&packet, thread);
^
This is due to the inclusion of the skip_rcv_packet() in the
CONFIG_PRESCAN_RXQ ifdef block. This function is independent of
CONFIG_PRESCAN_RXQ and should be outside this block.
Fixes: 82c2611daaf0 ("staging/rdma/hfi1: Handle packets with invalid RHF on
context 0")
Reviewed-by: Mike Marciniszyn
Signed-off-by: Jubin John
---
drivers/staging/rdma/hfi1/driver.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/staging/rdma/hfi1/driver.c
b/drivers/staging/rdma/hfi1/driver.c
index 4c52e78..3b7a707 100644
--- a/drivers/staging/rdma/hfi1/driver.c
+++ b/drivers/staging/rdma/hfi1/driver.c
@@ -633,6 +633,7 @@ next:
update_ps_mdata(&mdata, rcd);
}
}
+#endif /* CONFIG_PRESCAN_RXQ */
static inline int skip_rcv_packet(struct hfi1_packet *packet, int thread)
{
@@ -659,7 +660,6 @@ static inline int skip_rcv_packet(struct hfi1_packet
*packet, int thread)
return ret;
}
-#endif /* CONFIG_PRESCAN_RXQ */
static inline int process_rcv_packet(struct hfi1_packet *packet, int thread)
{
--
1.7.1
- End forwarded message -
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 0/2] staging/rdma/hfi1: set Gen 3 half-swing for integrated devices.
Any further feedback on this series? Ira On Tue, Dec 01, 2015 at 02:47:55PM -0500, ira.we...@intel.com wrote: > From: Ira Weiny > > This was a single patch before. The change to dev_dbg required a precursor > patch to add the dd_dev_dbg which is consistent with the other dev_* macros > which automatically use struct hfi1_devdata. > > Dean Luick (1): > staging/rdma/hfi1: set Gen3 half-swing for integrated devices > > Ira Weiny (1): > staging/rdma/hf1: add dd_dev_dbg > > drivers/staging/rdma/hfi1/chip_registers.h | 11 > drivers/staging/rdma/hfi1/hfi.h| 4 ++ > drivers/staging/rdma/hfi1/pcie.c | 82 > -- > 3 files changed, 93 insertions(+), 4 deletions(-) > > -- > 1.8.2 > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/14] staging/rdma/hfi1: Start adding building blocks for TID caching
On Thu, Dec 17, 2015 at 02:59:52PM +0300, Dan Carpenter wrote:
> On Thu, Dec 17, 2015 at 02:00:18AM -0500, ira.we...@intel.com wrote:
> > +}
> > +
> > +static int program_rcvarray(struct file *fp, unsigned long vaddr,
> > + struct tid_group *grp,
> > + struct tid_pageset *sets,
> > + unsigned start, u16 count, struct page **pages,
> > + u32 *tidlist, unsigned *tididx, unsigned *pmapped)
> > +{
>
> It's not clear what a zero return from this function means. Could we
> add some documentation?
Added the following.
/**
* program_rcvarray() - program an RcvArray group with receive buffers
* @fp: file pointer
* @vaddr: starting user virtual address
* @grp: RcvArray group
* @sets: array of struct tid_pageset holding information on physically
*contiguous chunks from the user buffer
* @start: starting index into sets array
* @count: number of struct tid_pageset's to program
* @pages: an array of struct page * for the user buffer
* @tidlist: the array of u32 elements when the information about the
* programmed RcvArray entries is to be encoded.
* @tididx: starting offset into tidlist
* @pmapped: (output parameter) number of pages programmed into the RcvArray
* entries.
*
* This function will program up to 'count' number of RcvArray entries from the
* group 'grp'. To make best use of write-combining writes, the function will
* perform writes to the unused RcvArray entries which will be ignored by the
* HW. Each RcvArray entry will be programmed with a physically contiguous
* buffer chunk from the user's virtual buffer.
*
* Return:
* -EINVAL if the requested count is larger than the size of the group,
* -ENOMEM or -EFAULT on error from set_rcvarray_entry(), or
* number of RcvArray entries programmed.
*/
Ira
>
> regards,
> dan carpenter
>
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 13/14] staging/rdma/hfi1: Add TID entry program function body
On Thu, Dec 17, 2015 at 03:06:45PM +0300, Dan Carpenter wrote:
> On Thu, Dec 17, 2015 at 02:00:23AM -0500, ira.we...@intel.com wrote:
> > + mutex_lock(&uctxt->exp_lock);
> > + /*
> > +* The first step is to program the RcvArray entries which are complete
> > +* groups.
> > +*/
> > + while (ngroups && uctxt->tid_group_list.count) {
> > + struct tid_group *grp =
> > + tid_group_pop(&uctxt->tid_group_list);
> > +
> > + ret = program_rcvarray(fp, vaddr, grp, pagesets,
> > + pageidx, dd->rcv_entries.group_size,
> > + pages, tidlist, &tididx, &mapped);
> > + /*
> > +* If there was a failure to program the RcvArray
> > +* entries for the entire group, reset the grp fields
> > +* and add the grp back to the free group list.
> > +*/
> > + if (ret <= 0) {
> > + tid_group_add_tail(grp, &uctxt->tid_group_list);
> > + hfi1_cdbg(TID,
> > + "Failed to program RcvArray group %d", ret);
> > + goto unlock;
>
> We print a failure message but we still return success when ret == 0.
Not programming anything (return of 0) is allowed. So it should not be an
error here. User space is required to handle this case.
Ira
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 08/14] staging/rdma/hfi1: Start adding building blocks for TID caching
On Thu, Dec 17, 2015 at 02:23:39PM +0300, Dan Carpenter wrote:
> On Thu, Dec 17, 2015 at 02:00:18AM -0500, ira.we...@intel.com wrote:
> > From: Mitko Haralanov
> > +static int unprogram_rcvarray(struct file *fp, u32 tidinfo,
> > + struct tid_group **grp)
> > +{
> > + struct hfi1_filedata *fd = fp->private_data;
> > + struct hfi1_ctxtdata *uctxt = fd->uctxt;
> > + struct hfi1_devdata *dd = uctxt->dd;
> > + struct mmu_rb_node *node;
> > + u8 tidctrl = EXP_TID_GET(tidinfo, CTRL);
> > + u32 tidbase = uctxt->expected_base,
> > + tididx = EXP_TID_GET(tidinfo, IDX) << 1, rcventry;
> > +
> > + if (tididx > uctxt->expected_count) {
>
> Should this be >= ? I don't think it makes that much difference since
> we're not using it as an offset.
Yes it looks like it should be.
I'm working on a V2 now.
Ira
>
> > + dd_dev_err(dd, "Invalid RcvArray entry (%u) index for ctxt
> > %u\n",
> > + tididx, uctxt->ctxt);
> > + return -EINVAL;
> > + }
> > +
> > + if (tidctrl == 0x3)
> > + return -EINVAL;
> > +
> > + rcventry = tidbase + tididx + (tidctrl - 1);
> > +
> > + spin_lock(&fd->rb_lock);
> > + node = mmu_rb_search_by_entry(&fd->tid_rb_root, rcventry);
> > + if (!node) {
> > + spin_unlock(&fd->rb_lock);
> > + return -EBADF;
> > + }
> > + rb_erase(&node->rbnode, &fd->tid_rb_root);
> > + spin_unlock(&fd->rb_lock);
> > + if (grp)
> > + *grp = node->grp;
> > + clear_tid_node(fd, fd->subctxt, node);
> > + return 0;
> > +}
>
> regards,
> dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/15] staging/rdma/hfi1: Initial patches to add rdmavt support in HFI1
Greg, Doug, As mentioned below, these patches depend on the new rdmavt library submitted to Doug on linux-rdma. We continue to identify (and rework) patches by our other developers which can be submitted without conflicts with this series. Furthermore, We have, as much as possible, placed fixes directly into rdmavt such that those changes can be dropped from hfi1. But at this point, we need to know if and where these are going to land so that we can start reworking as appropriate. Therefore, I would like to discuss plans to get hfi1 under the same maintainer to work through this transitional period. Basically, At what point should we stop submitting patches to Greg and start submitting to Doug? Should we consider the merge window itself as the swap over point and submit changes to Doug at that point? If so, should we continue to submit what we can to Greg until then (and continue rebase'ing the series below on that work)? Or given Gregs backlog, should we stop submitting to Greg sometime prior to the merge window? That brings up my final question, at the point of swap over I assume anything not accepted by Greg should be considered rejected and we need to resubmit to Doug? Thanks in advance for any guidance, Ira On Mon, Dec 14, 2015 at 12:27:49PM -0500, Dalessandro, Dennis wrote: > This patch series is being submitted as a Request For Comment only. It depends > on code submitted to the rdma subsystem [1]. > > This work is the first submission aimed to satisfy the TODO item for removing > duplicated code in hfi1. At the time of submission hfi1 and qib contained alot > of duplicated verbs processing code. The qib driver is having similar changes > made to use rdmavt. This will result in a common code base that both drivers > and > future drivers such as soft-roce can use. > > Note that due to the ongoing submission of hfi1 improvement patches, there > will > likely be a number of conflicts which will still need to be resolved. > > We also are still faced with the issue of separate trees for this work as was > discussed previously [2]. The result of that conversation was to keep the > drivers in separate trees until the 4.5 merge window. We are hoping that after > this merge window a single maintainer can take control of hfi1, qib, and > rdmavt > so that these patches can move forward and be applied. > > For now though we would like to get feedback on these patches with more to > follow. > > [1] https://www.mail-archive.com/linux-rdma@vger.kernel.org/msg30074.html > [2] https://www.mail-archive.com/linux-rdma%40vger.kernel.org/msg29360.html > > --- > > Dennis Dalessandro (15): > IB/hfi1: Begin to use rdmavt for verbs > IB/hfi1: Add basic rdmavt capability flags for hfi1 > IB/hfi1: Consolidate dma ops for hfi1 > IB/hfi1: Use rdmavt protection domain > IB/hfi1: Remove MR data structures from hfi1 > IB/hfi1: Remove driver specific members from hfi1 qp type > IB/hfi1: Add device specific info prints > IB/hfi1: Use correct rdmavt header files after move. > IB/hfi1: Use address handle in rdmavt and remove from hfi1 > IB/hfi1: Implement hfi1 support for AH notification > IB/hfi1: Remove hfi1 MR and hfi1 specific qp type > IB/hfi1: Remove srq from hfi1 > IB/hfi1: Remove ibport and use rdmavt version > IB/hfi1: Remove mmap from hfi1 > IB/hfi1: Use rdmavt pkey verbs function > > > drivers/staging/rdma/hfi1/Kconfig |2 > drivers/staging/rdma/hfi1/Makefile |4 > drivers/staging/rdma/hfi1/chip.c| 36 +- > drivers/staging/rdma/hfi1/common.h |2 > drivers/staging/rdma/hfi1/cq.c | 20 + > drivers/staging/rdma/hfi1/diag.c| 13 - > drivers/staging/rdma/hfi1/driver.c | 31 +- > drivers/staging/rdma/hfi1/hfi.h | 27 +- > drivers/staging/rdma/hfi1/init.c|5 > drivers/staging/rdma/hfi1/intr.c|2 > drivers/staging/rdma/hfi1/keys.c| 356 - > drivers/staging/rdma/hfi1/mad.c | 163 +- > drivers/staging/rdma/hfi1/mmap.c| 192 --- > drivers/staging/rdma/hfi1/mr.c | 522 -- > drivers/staging/rdma/hfi1/pio.c | 10 - > drivers/staging/rdma/hfi1/qp.c | 214 +++- > drivers/staging/rdma/hfi1/qp.h | 44 +-- > drivers/staging/rdma/hfi1/rc.c | 155 + > drivers/staging/rdma/hfi1/ruc.c | 161 + > drivers/staging/rdma/hfi1/sdma.h|8 > drivers/staging/rdma/hfi1/srq.c | 58 ++- > drivers/staging/rdma/hfi1/sysfs.c | 18 + > drivers/staging/rdma/hfi1/trace.h | 22 + > drivers/staging/rdma/hfi1/uc.c | 19 + > drivers/staging/rdma/hfi1/ud.c | 91 +++-- > drivers/staging/rdma/hfi1/verbs.c | 526 > +++ > drivers/staging/rdma/hfi1/verbs.h | 531 > ---
Re: [PATCH v2 0/5] Clean up SDMA engine code
On Tue, Dec 08, 2015 at 05:10:08PM -0500, ira.we...@intel.com wrote: > From: Ira Weiny > > Various improvements to the SDMA engine code. Greg, Thanks for reviewing and accepting our patches to staging-testing. I apologize for the conflicts we had between the 3 of us submitting. However, in attempting to rework an internal branch to ensure this does not happen again I believe there were more conflicts than their should have been due to patches being accepted out of order. For example, I found the following error in your staging tree below. This series you applied in the following order which causes a build failure on the middle commit -- a0d4069. 483119a staging/rdma/hfi1: Unconditionally clean-up SDMA queues def8228 staging/rdma/hfi1: Convert to use get_user_pages_fast a0d4069 staging/rdma/hfi1: Add page lock limit check for SDMA requests faa98b8 staging/rdma/hfi1: Clean-up unnecessary goto statements 6a5464f staging/rdma/hfi1: Detect SDMA transmission error early The order as submitted was: staging/rdma/hfi1: Convert to use get_user_pages_fast staging/rdma/hfi1: Unconditionally clean-up SDMA queues staging/rdma/hfi1: Clean-up unnecessary goto statements staging/rdma/hfi1: Detect SDMA transmission error early staging/rdma/hfi1: Add page lock limit check for SDMA requests Do I need to resolve this somehow? Or is this something you resolve while the patches are in staging-testing? Is there something we need to do in the cover letter of a patch series to ensure order? Perhaps my cover letter implied these were not ordered? If so, I again apologize. Thanks, Ira > > --- > Changes from V1: > Fix off by one error in the last patch > > Mitko Haralanov (5): > staging/rdma/hfi1: Convert to use get_user_pages_fast > staging/rdma/hfi1: Unconditionally clean-up SDMA queues > staging/rdma/hfi1: Clean-up unnecessary goto statements > staging/rdma/hfi1: Detect SDMA transmission error early > staging/rdma/hfi1: Add page lock limit check for SDMA requests > > drivers/staging/rdma/hfi1/file_ops.c | 11 +- > drivers/staging/rdma/hfi1/hfi.h| 4 +- > drivers/staging/rdma/hfi1/user_pages.c | 97 +++--- > drivers/staging/rdma/hfi1/user_sdma.c | 319 > +++-- > drivers/staging/rdma/hfi1/user_sdma.h | 2 + > 5 files changed, 222 insertions(+), 211 deletions(-) > > -- > 1.8.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/15] staging/rdma/hfi1: Initial patches to add rdmavt support in HFI1
On Mon, Dec 21, 2015 at 02:02:35PM -0800, gre...@linuxfoundation.org wrote: > On Mon, Dec 21, 2015 at 01:12:14AM -0500, ira.weiny wrote: > > Greg, Doug, > > > > As mentioned below, these patches depend on the new rdmavt library > > submitted to > > Doug on linux-rdma. > > > > We continue to identify (and rework) patches by our other developers which > > can > > be submitted without conflicts with this series. Furthermore, We have, as > > much > > as possible, placed fixes directly into rdmavt such that those changes can > > be > > dropped from hfi1. But at this point, we need to know if and where these > > are > > going to land so that we can start reworking as appropriate. > > > > Therefore, I would like to discuss plans to get hfi1 under the same > > maintainer > > to work through this transitional period. > > > > Basically, At what point should we stop submitting patches to Greg and start > > submitting to Doug? > > > > Should we consider the merge window itself as the swap over point and submit > > changes to Doug at that point? If so, should we continue to submit what we > > can > > to Greg until then (and continue rebase'ing the series below on that work)? > > Or > > given Gregs backlog, should we stop submitting to Greg sometime prior to the > > merge window? > > > > That brings up my final question, at the point of swap over I assume > > anything > > not accepted by Greg should be considered rejected and we need to resubmit > > to > > Doug? > > If Doug accepts the library changes, let me know that public git commit > and I can pull it into the staging-next branch and you can continue to > send me staging patches that way. Won't this cause a conflict during the merge window? How do we handle changes which affect both qib and hfi1? Ira > > That's the easiest thing to do usually. > > thanks, > > greg k-h > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 0/5] Clean up SDMA engine code
On Mon, Dec 21, 2015 at 04:13:49PM -0800, gre...@linuxfoundation.org wrote: > On Mon, Dec 21, 2015 at 06:48:03PM -0500, ira.weiny wrote: > > On Tue, Dec 08, 2015 at 05:10:08PM -0500, ira.we...@intel.com wrote: > > > From: Ira Weiny > > > > > > Various improvements to the SDMA engine code. > > > > Greg, > > > > Thanks for reviewing and accepting our patches to staging-testing. I > > apologize > > for the conflicts we had between the 3 of us submitting. However, in > > attempting to rework an internal branch to ensure this does not happen > > again I > > believe there were more conflicts than their should have been due to patches > > being accepted out of order. > > > > For example, I found the following error in your staging tree below. > > > > This series you applied in the following order which causes a build failure > > on > > the middle commit -- a0d4069. > > > > 483119a staging/rdma/hfi1: Unconditionally clean-up SDMA queues > > def8228 staging/rdma/hfi1: Convert to use get_user_pages_fast > > a0d4069 staging/rdma/hfi1: Add page lock limit check for SDMA requests > > faa98b8 staging/rdma/hfi1: Clean-up unnecessary goto statements > > 6a5464f staging/rdma/hfi1: Detect SDMA transmission error early > > > > The order as submitted was: > > > > staging/rdma/hfi1: Convert to use get_user_pages_fast > > staging/rdma/hfi1: Unconditionally clean-up SDMA queues > > staging/rdma/hfi1: Clean-up unnecessary goto statements > > staging/rdma/hfi1: Detect SDMA transmission error early > > staging/rdma/hfi1: Add page lock limit check for SDMA requests > > > > > > > > Do I need to resolve this somehow? Or is this something you resolve while > > the > > patches are in staging-testing? > > > > Is there something we need to do in the cover letter of a patch series to > > ensure order? Perhaps my cover letter implied these were not ordered? If > > so, > > I again apologize. > > Did you number your patches? Yes, sent with git-send-email. http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-December/thread.html#82509 > That's the only way to ensure that they > are applied in the correct order, that's what I sort on to apply them. > If you don't order them, I randomly guess, or just reject them... > > All seems to build now, right? Yes all builds now. I just did not know if as part of testing an incremental build check would then reject the patch. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/15] staging/rdma/hfi1: Initial patches to add rdmavt support in HFI1
On Mon, Dec 21, 2015 at 05:01:48PM -0800, gre...@linuxfoundation.org wrote: > On Mon, Dec 21, 2015 at 07:19:43PM -0500, ira.weiny wrote: > > On Mon, Dec 21, 2015 at 02:02:35PM -0800, gre...@linuxfoundation.org wrote: > > > On Mon, Dec 21, 2015 at 01:12:14AM -0500, ira.weiny wrote: > > > > Greg, Doug, > > > > > > > > As mentioned below, these patches depend on the new rdmavt library > > > > submitted to > > > > Doug on linux-rdma. > > > > > > > > We continue to identify (and rework) patches by our other developers > > > > which can > > > > be submitted without conflicts with this series. Furthermore, We have, > > > > as much > > > > as possible, placed fixes directly into rdmavt such that those changes > > > > can be > > > > dropped from hfi1. But at this point, we need to know if and where > > > > these are > > > > going to land so that we can start reworking as appropriate. > > > > > > > > Therefore, I would like to discuss plans to get hfi1 under the same > > > > maintainer > > > > to work through this transitional period. > > > > > > > > Basically, At what point should we stop submitting patches to Greg and > > > > start > > > > submitting to Doug? > > > > > > > > Should we consider the merge window itself as the swap over point and > > > > submit > > > > changes to Doug at that point? If so, should we continue to submit > > > > what we can > > > > to Greg until then (and continue rebase'ing the series below on that > > > > work)? Or > > > > given Gregs backlog, should we stop submitting to Greg sometime prior > > > > to the > > > > merge window? > > > > > > > > That brings up my final question, at the point of swap over I assume > > > > anything > > > > not accepted by Greg should be considered rejected and we need to > > > > resubmit to > > > > Doug? > > > > > > If Doug accepts the library changes, let me know that public git commit > > > and I can pull it into the staging-next branch and you can continue to > > > send me staging patches that way. > > > > Won't this cause a conflict during the merge window? > > No, git is good :) > > > How do we handle changes which affect both qib and hfi1? > > I don't know, now this gets messy... > Agreed and this is what we are worried about. Can we do what Dan and Doug have proposed in the past and have Doug take over the staging/rdma sub-tree? http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-November/081922.html I think the upcoming merge window is a reasonable time for him to do that. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/15] staging/rdma/hfi1: Initial patches to add rdmavt support in HFI1
On Tue, Dec 22, 2015 at 06:27:57PM -0800, gre...@linuxfoundation.org wrote: > On Tue, Dec 22, 2015 at 02:15:08PM -0500, ira.weiny wrote: > > On Mon, Dec 21, 2015 at 05:01:48PM -0800, gre...@linuxfoundation.org wrote: [snip] > > > > > > No, git is good :) > > > > > > > How do we handle changes which affect both qib and hfi1? > > > > > > I don't know, now this gets messy... > > > > > > > Agreed and this is what we are worried about. > > > > Can we do what Dan and Doug have proposed in the past and have Doug take > > over > > the staging/rdma sub-tree? > > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-November/081922.html > > > > I think the upcoming merge window is a reasonable time for him to do that. > > Ok, but keeping on top of all of the generic staging patches that come > in is a tough thing to do, that's up to Doug, if he is ready for it... > To help this process, once the change over happens, we will help to monitor driverdev-devel for anything submitted to staging/rdma. If something is submitted which was not to Doug and linux-rdma we can handle alerting the submitter to make sure it gets submitted to Doug as per the current MAINTAINERS file. Hope this helps, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [RFC PATCH 00/15] staging/rdma/hfi1: Initial patches to add rdmavt support in HFI1
> > > > > > > > > > If Doug accepts the library changes, let me know that public git > > > > > commit > > > > > and I can pull it into the staging-next branch and you can continue to > > > > > send me staging patches that way. > > > > > > > > Won't this cause a conflict during the merge window? > > > > > > No, git is good :) > > > > > > > How do we handle changes which affect both qib and hfi1? > > > > > > I don't know, now this gets messy... > > > > > > > Agreed and this is what we are worried about. > > > > Can we do what Dan and Doug have proposed in the past and have Doug take > > over > > the staging/rdma sub-tree? > > > > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2015-November/081922.html > > > > I think the upcoming merge window is a reasonable time for him to do that. > > Ok, but keeping on top of all of the generic staging patches that come > in is a tough thing to do, that's up to Doug, if he is ready for it... > Greg, Forgive me for not knowing how multiple maintainers deal with hand offs like this. I'm hoping you, and/or Doug, can answer some questions for me. Am I correct in assuming the merge window will be open on Monday? If so, when will Linus pull the staging tree? Then at what point will Doug get the hfi1 changes which have already been accepted? Are you going to be able to review the outstanding patches for staging/rdma/hfi1 before the merge window? Or should we consider them dropped and resubmit to Doug to apply after he has merged the latest hfi1 code from Linus? Doug, At what point should we start submitting additional patches to you which we have queued up but are not yet submitted to Greg? So far we have been cross posting to linux-rdma for feedback and I see that the patches have been dropped from patchworks. I assume you dropped them from patchworks because you knew that Greg was handling them? Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Missed patch for hfi1 from Gregs tree.
Doug, The following patch was not accepted by Greg and does not appear in patchworks https://marc.info/?l=linux-rdma&m=145090416416677&w=2 We have this patch and can resubmit it for patchworks but I don't want to get in the way of the original author. Julia, Would you mind if we resubmit it to get it in the queue for Doug? Otherwise could you resubmit? Note: the staging/rdma tree is now being maintained by Doug Ledford (linux-rdma). Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: Missed patch for hfi1 from Gregs tree.
On Thu, Jan 28, 2016 at 09:52:09PM +0100, Julia Lawall wrote: > > > On Thu, 28 Jan 2016, gre...@linuxfoundation.org wrote: > > > On Thu, Jan 28, 2016 at 03:30:23PM -0500, ira.weiny wrote: > > > Doug, > > > > > > The following patch was not accepted by Greg and does not appear in > > > patchworks > > > > > > https://marc.info/?l=linux-rdma&m=145090416416677&w=2 > > > > I have a whole stack of staging patches I have yet to get through. Over > > 1000 at the moment, they will be gotten to eventually... Greg, As we agreed to here: http://www.spinics.net/lists/linux-rdma/msg31782.html Doug has taken over the staging/rdma tree. I offered to help direct submissions to the staging tree to Doug/linux-rdma so that you would not have to worry about them. That is what I am doing here. Therefore, we would like Julia to resubmit to Doug so that we have 1 maintainer for this driver and the rdmavt/qib work. > > > > greg k-h > > Should I resubmit? Yes please resubmit to Doug/linux-rdma. Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: rdma: use setup_timer() api
On Thu, Feb 04, 2016 at 09:38:32AM -0600, Mike Marciniszyn wrote: > > Subject: [PATCH] staging: rdma: use setup_timer() api > > > > From: Hari Prasath Gujulan Elango > > > > Doug, > > We are going to add this to the rdmavt/hfi1/qib work, with the appropriate > authorship credit. Otherwise, there will certainly be conflicts. > > Hari, > > Note that ehca is going away. Actually ehca is already gone. From Dougs for-4.5-rc tree. 12:30:25 > ls Kconfig Makefile hfi1/ Ira > > In addition, you missed exploiting the third argument to setup_timer() for > the callback context. > > Mike ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 00/23] Update driver to 0.9-294
On Tue, Oct 20, 2015 at 03:45:47PM +0300, Moni Shoua wrote: > > > > Perhaps I did not chose my words carefully enough. > > > > The largest issue on the TODO list is the refactoring of the code to be > > shared between the hfi1 and qib driver. While the hardware between hfi1 > > and qib is similar and thus the initial code looked similar, our > > performance tuning on hfi1 has revealed that some changes will be required > > to the hfi1 code to fully utilize the hardware. We would like to get these > > updates in to make sure that the refactoring work is done properly for the > > 2 hardware types. > > Please keep in mind that there are 3 HW types (our SoftRoCE driver) > that needs to be part of the framework. Understood, however, unlike SoftRoCE, qib and hfi currently share a lot of code to drive the hardware. The underlying reason for the TODO item "Remove software processing of IB protocol..." is because we have a large amount of duplicated code between these drivers. _Some_ of which, at a high level, is sharable with SoftRoCE. These patches (and more to follow), further differentiate the 2 drivers along hardware lines. Accepting these patches will help us make sure that we don't create some common code between qib and hfi which, because of our testing we now know, needs to be separated out. This is a separate issue from the higher level code sharing which will be done with SoftRoCE. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 15/22] staging/rdma/hfi1: Allow tuning of SDMA interrupt rate
On Thu, Oct 22, 2015 at 01:54:21PM +0300, Dan Carpenter wrote: > What values work well for this parameter? 64. For v3, I've squashed a change which was farther down my list of changes which set this after some testing. Thanks, Ira > > regards, > dan carpenter > ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 14/22] staging/rdma/hfi1: Implement Expected Receive TID caching
On Thu, Oct 22, 2015 at 01:41:58PM +0300, Dan Carpenter wrote:
> On Mon, Oct 19, 2015 at 10:11:29PM -0400, ira.we...@intel.com wrote:
> > + case HFI1_CMD_TID_INVAL_READ:
> > + ret = hfi1_user_exp_rcv_invalid(fp, &tinfo);
> > + if (!ret) {
> > + addr = (unsigned long)cmd.addr +
> > + offsetof(struct hfi1_tid_info, tidcnt);
> > + if (copy_to_user((void __user *)addr, &tinfo.tidcnt,
> > +sizeof(tinfo.tidcnt)))
> > + ret = -EFAULT;
> > + }
> > + break;
>
> This switch statement uses success handling throughtout instead of
> error handling. It's better if you write it like this:
>
> case HFI1_CMD_TID_INVAL_READ:
> ret = hfi1_user_exp_rcv_invalid(fp, &tinfo);
> if (ret)
> break;
>
> addr = (unsigned long)cmd.addr +
> offsetof(struct hfi1_tid_info, tidcnt);
> if (copy_to_user((void __user *)addr, &tinfo.tidcnt,
>sizeof(tinfo.tidcnt)))
> ret = -EFAULT;
> break;
This follows the rest of the style of the case statement in this function. We
prefer to leave this as is for a number of reasons.
1) This is consistent with the coding style elsewhere in this driver.
2) It is functionally equivalent.
3) I have a long list of patches which need to be processed and this may cause
later merge conflicts.
The main reason is number 1. We want to remain consistent within this driver.
>
>
> The casting is sort of ugly... It would be better to make address a
> pointer. It does cut down on the lines of code but at least the cast is
> all done at once and really "addr" is actually a pointer.
>
> case HFI1_CMD_TID_INVAL_READ:
> ret = hfi1_user_exp_rcv_invalid(fp, &tinfo);
> if (ret)
> break;
>
> addr = (void __user *)(unsigned long)cmd.addr +
> offsetof(struct hfi1_tid_info, tidcnt);
> if (copy_to_user(addr, &tinfo.tidcnt, sizeof(tinfo.tidcnt)))
> ret = -EFAULT;
> break;
I think this is more a matter of style.
>
>
> > case HFI1_CMD_TID_FREE:
> > - ret = exp_tid_free(fp, &tinfo);
> > + ret = hfi1_user_exp_rcv_clear(fp, &tinfo);
> > + addr = (unsigned long)cmd.addr +
> > + offsetof(struct hfi1_tid_info, tidcnt);
> > + if (copy_to_user((void __user *)addr, &tinfo.tidcnt,
> > +sizeof(tinfo.tidcnt)))
> > + ret = -EFAULT;
> > break;
>
> This is an information leak. We should break if
> hfi1_user_exp_rcv_clear() fails, but instead we copy uninitialized
> variables to the user.
That is a bug, thanks.
Fixed in v3 (Although, I did use the same positive error check style to be
consistent within this function.)
>
>
>
> > case HFI1_CMD_RECV_CTRL:
> > ret = manage_rcvq(uctxt, subctxt_fp(fp), (int)user_val);
> > @@ -607,9 +603,9 @@ static int hfi1_file_mmap(struct file *fp, struct
> > vm_area_struct *vma)
> > * Use the page where this context's flags are. User level
> > * knows where it's own bitmap is within the page.
> > */
> > - memaddr = ((unsigned long)dd->events +
> > - ((uctxt->ctxt - dd->first_user_ctxt) *
> > - HFI1_MAX_SHARED_CTXTS)) & PAGE_MASK;
> > + memaddr = (unsigned long)(dd->events +
> > + ((uctxt->ctxt - dd->first_user_ctxt) *
> > + HFI1_MAX_SHARED_CTXTS)) & PAGE_MASK;
>
> I am too lazy to investigate the types of all these variables but I'm
> instead going to assert that moving the cast to later is an unrelated
> white space change. Don't mix white space changes in with a behavior
> change patch because it makes it hard to review.
Actually this is a bug fix. dd->events is a pointer which needs to be indexed
prior to the cast. I'll split this into a separate patch in v3.
Thanks!
Ira
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2 01/22] staging/rdma/hfi1: Fix regression in send performance
On Wed, Oct 21, 2015 at 04:18:06PM +0300, Dan Carpenter wrote: > On Mon, Oct 19, 2015 at 10:11:16PM -0400, ira.we...@intel.com wrote: > > From: Mike Marciniszyn > > > > This additional call is a regression from qib. For small messages the > > progress > > routine always builds one and clears out the ahg state when the queue has > > gone > > to empty which is the predominant case for small messages. > > > > Inline the routine to mitigate the call and move the routine to qp.h for > > scope > > reasons. > > > > Reviewed-by: Dennis Dalessandro > > Signed-off-by: Mike Marciniszyn > > Signed-off-by: Ira Weiny > > --- > > The whole point of this patch is to do: > > > - if (qp->s_sde) > > + if (qp->s_sde && qp->s_ahgidx >= 0) > > It was not at all clear from the changelog and it was difficult to tell > because we moved code around as well. I've cleaned up the commit message. v3 should be on its way after I make sure I have addressed all your comments on all the patches. Ira > > regards, > dan carpenter ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 12/23] staging/rdma/hfi1: Macro code clean up
On Tue, Oct 27, 2015 at 05:19:10PM +0900, Greg KH wrote:
> On Mon, Oct 26, 2015 at 10:28:38AM -0400, ira.we...@intel.com wrote:
> > From: Mitko Haralanov
> >
> > Clean up the context and sdma macros and move them to a more logical place
> > in
> > hfi.h
> >
> > Signed-off-by: Mitko Haralanov
> > Signed-off-by: Ira Weiny
> > ---
> > drivers/staging/rdma/hfi1/hfi.h | 22 ++
> > 1 file changed, 10 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/staging/rdma/hfi1/hfi.h
> > b/drivers/staging/rdma/hfi1/hfi.h
> > index a35213e9b500..41ad9a30149b 100644
> > --- a/drivers/staging/rdma/hfi1/hfi.h
> > +++ b/drivers/staging/rdma/hfi1/hfi.h
> > @@ -1104,6 +1104,16 @@ struct hfi1_filedata {
> > int rec_cpu_num;
> > };
> >
> > +/* for use in system calls, where we want to know device type, etc. */
> > +#define fp_to_fd(fp) ((struct hfi1_filedata *)(fp)->private_data)
> > +#define ctxt_fp(fp) (fp_to_fd((fp))->uctxt)
> > +#define subctxt_fp(fp) (fp_to_fd((fp))->subctxt)
> > +#define tidcursor_fp(fp) (fp_to_fd((fp))->tidcursor)
> > +#define user_sdma_pkt_fp(fp) (fp_to_fd((fp))->pq)
> > +#define user_sdma_comp_fp(fp) (fp_to_fd((fp))->cq)
> > +#define notifier_fp(fp) (fp_to_fd((fp))->mn)
> > +#define rb_fp(fp) (fp_to_fd((fp))->tid_rb_root)
>
> Ick, no, don't do this, just spell it all out (odds are you will see tht
> you can make the code simpler...) If you don't know what "cq" or "pq"
> are, then name them properly.
>
> These need to be all removed.
Ok.
Can I add the removal of these macros to the TODO list and get this patch
accepted in the interm?
Many of the patches I am queueing up to submit as well as one in this series do
not apply cleanly without this change. It will be much easier if I can get
everything applied and then do a global clean up of these macros after the
fact.
Thanks,
Ira
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 23/23] staging/rdma/hfi1: Update driver version string to 0.9-294
On Tue, Oct 27, 2015 at 05:46:41PM +0900, Greg KH wrote: > On Mon, Oct 26, 2015 at 10:28:49AM -0400, ira.we...@intel.com wrote: > > From: Jubin John > > > > Signed-off-by: Jubin John > > Signed-off-by: Ira Weiny > > --- > > drivers/staging/rdma/hfi1/common.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/rdma/hfi1/common.h > > b/drivers/staging/rdma/hfi1/common.h > > index 7809093eb55e..5dd92720faae 100644 > > --- a/drivers/staging/rdma/hfi1/common.h > > +++ b/drivers/staging/rdma/hfi1/common.h > > @@ -205,7 +205,7 @@ > > * to the driver itself, not the software interfaces it supports. > > */ > > #ifndef HFI1_DRIVER_VERSION_BASE > > -#define HFI1_DRIVER_VERSION_BASE "0.9-248" > > +#define HFI1_DRIVER_VERSION_BASE "0.9-294" > > Patches like this make no sense at all, please drop it and only use the > kernel version. What do you mean by "only use the kernel version"? Do you mean #define HFI1_DRIVER_VERSION_BASE UTS_RELEASE Or just remove the macro entirely? > > Trust me, it's going to get messy really fast (hint, it > already did...) Did I base this on the wrong tree? Not sure how this could have messed you up. Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v3 12/23] staging/rdma/hfi1: Macro code clean up
> > Can I add the removal of these macros to the TODO list and get this patch > > accepted in the interm? > > Nope, sorry, why would I accept a known-problem patch? Would you do > such a thing? > > > Many of the patches I am queueing up to submit as well as one in this > > series do > > not apply cleanly without this change. It will be much easier if I can get > > everything applied and then do a global clean up of these macros after the > > fact. > > But you would have no incentive to do that if I take this patch now :) > Understood. Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [BUG] IB/hfi1: might sleep under spinlock in hfi1_ioctl()
On Sat, Oct 31, 2015 at 12:32:29AM +0300, Alexey Khoroshilov wrote: > Hello, > > hfi1_ioctl() contains many calls to might sleep functions with > dd->hfi1_snoop.snoop_lock spinlock held (for example, access_ok, > copy_from_user, kzalloc(GFP_KERNEL), etc.). > > Should dd->hfi1_snoop.snoop_lock be acquired just before updating state? I believe you are correct. I am currently in the process of pushing fixes to the staging tree. We have a patch which fixes this queued up but it depends on at least one other patch in my queue. I will do my best to get this submitted soon. Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging/rdma/hfi1: set Gen3 half-swing for integrated devices
On Thu, Nov 05, 2015 at 10:34:48AM +0300, Dan Carpenter wrote: > On Wed, Nov 04, 2015 at 09:06:08PM -0500, ira.we...@intel.com wrote: > > > +#define CCE_PCIE_CTRL_XMT_MARGIN_GEN1_GEN2_OVERWRITE_ENABLE_MASK 0x1ull > > There is absolutely no point to having a variable name which is nine > bajillion characters long since we are not able to use it and have to > instead use macro majic to hide the first twelve zillion characters. > The long names are directly from the hardware specification. > > If > we can't even see the name when it is used, then why does it exist? Also > macro magic breaks grep and cscope. Just think of a better name to > begin with and get rid of the PC macro. We did not want the PC macro but using the "real" names caused checkpatch failures. Indeed using the PC macro reduces the readability of the code in terms of following the hardware spec because we lose the "real" name. Is this an example where we should ignore the line lengths of checkpatch to preserve the readability of the code? Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 3/3] staging/rdma/hfi1: Method to toggle "fast ECN" detection
On Thu, Nov 05, 2015 at 10:56:27AM +0300, Dan Carpenter wrote: > On Wed, Nov 04, 2015 at 09:10:11PM -0500, ira.we...@intel.com wrote: > > From: Vennila Megavannan > > > > Add a module paramter to toggle prescan/Fast ECN Detection. > > > > In addition change the PRESCAN_RXQ Kconfig default to "yes". > > > > Reviewed-by: Arthur Kepner > > Reviewed-by: Mike Marciniszyn > > Signed-off-by: Vennila Megavannan > > Signed-off-by: Ira Weiny > > Hm... In the original code we had the config entry but the code to > support this was actually disabled. Now we are turning on the config > entry by default but the module parameter defaults to disabled. I think > the documentation is a bit tricky because it should say that actually > enabling the config is not enough, it's still disabled. We tried to make it clear but obviously missed the mark. > > Do we really need the config to be there? Distros are going to enable it. > Who is it who wants to disable the config? The config is maintained to be able to fully remove the code to obtain the maximum receive performance _if_ the customer knows that they will never need the prescanning. This is the _ultimate_ in performance on this path. We anticipate some HPC customers who build their own kernels may want this option. > > Also is it better to > default to off or on for this code, what are the upsides and downsides > of that choice? > I'll update the commit message. But will also explain here. Enabling the code with the config option introduces a _slight_ performance impact but allows the user to determine if this congestion control fix should be active or not at run time. You are correct that most distros will enable the config options (default Yes). Therefore this becomes a system admin control item for the fabric being configured. After testing we found that this was the best compromise for most systems in terms of performance and flexibility. This is because prescanning is not required most of the time. Should a customer find that their topology and/or workload requires prescanning they can then enable this option at run time. This will result in a significant reduction of performance at the node level but may for those customers result in better overall fabric/cluster performance. So in conclusion we have 3 performance levels and the combination in this patch puts us at the "middle one". Thanks, Ira ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/1] staging: rdma: hfi1 : Prefer using the BIT macro
On Thu, Nov 05, 2015 at 05:28:03PM +0530, Sunny Kumar wrote: > This patch replaces bit shifting on 1 with the BIT(x) macro > > Signed-off-by: Sunny Kumar Also, NAK as has been covered in other responses. However, I wanted to add, similar to the hfi1_ioctl fix, we have follow on checkpatch patches which address this. Ira > --- > drivers/staging/rdma/hfi1/user_sdma.c | 10 +- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/staging/rdma/hfi1/user_sdma.c > b/drivers/staging/rdma/hfi1/user_sdma.c > index 36c838d..95a6d99 100644 > --- a/drivers/staging/rdma/hfi1/user_sdma.c > +++ b/drivers/staging/rdma/hfi1/user_sdma.c > @@ -146,8 +146,8 @@ MODULE_PARM_DESC(sdma_comp_size, "Size of User SDMA > completion ring. Default: 12 > #define KDETH_OM_MAX_SIZE (1 << ((KDETH_OM_LARGE / KDETH_OM_SMALL) + 1)) > > /* Last packet in the request */ > -#define TXREQ_FLAGS_REQ_LAST_PKT (1 << 0) > -#define TXREQ_FLAGS_IOVEC_LAST_PKT (1 << 0) > +#define TXREQ_FLAGS_REQ_LAST_PKT BIT(1 << 0) > +#define TXREQ_FLAGS_IOVEC_LAST_PKT BIT(1 << 0) > > #define SDMA_REQ_IN_USE 0 > #define SDMA_REQ_FOR_THREAD 1 > @@ -156,9 +156,9 @@ MODULE_PARM_DESC(sdma_comp_size, "Size of User SDMA > completion ring. Default: 12 > #define SDMA_REQ_HAS_ERROR 4 > #define SDMA_REQ_DONE_ERROR 5 > > -#define SDMA_PKT_Q_INACTIVE (1 << 0) > -#define SDMA_PKT_Q_ACTIVE (1 << 1) > -#define SDMA_PKT_Q_DEFERRED (1 << 2) > +#define SDMA_PKT_Q_INACTIVE BIT(1 << 0) > +#define SDMA_PKT_Q_ACTIVE BIT(1 << 1) > +#define SDMA_PKT_Q_DEFERRED BIT(1 << 2) > > /* > * Maximum retry attempts to submit a TX request > -- > 2.1.4 > > ___ > devel mailing list > de...@linuxdriverproject.org > http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
