Re: [openib-general] Fix some suspicious ppc64 code in dapl
Hi, > Thank you Anton. Could you replying with a signed off by line? I'll > properly attribute this fix to you in the commit log. Sorry, I let this one slip. Here it is. Anton -- I was reading through the ppc64 specific code in dapl/ and noticed some suspicious inline assembly. - EIEIO_ON_SMP and ISYNC_ON_SMP are in kernel UP build optimisations, we shouldnt export them to userspace. Replace it with lwsync and isync. - The comment says its implemenenting cmpxchg64 but in fact its implementing cmpxchg32. Fix the comment. Signed-off-by: Anton Blanchard <[EMAIL PROTECTED]> Index: dapl/udapl/linux/dapl_osd.h === --- dapl/udapl/linux/dapl_osd.h (revision 7621) +++ dapl/udapl/linux/dapl_osd.h (working copy) @@ -238,14 +238,13 @@ #endif /* __ia64__ */ #elif defined(__PPC64__) __asm__ __volatile__ ( -EIEIO_ON_SMP -"1: lwarx %0,0,%2 # __cmpxchg_u64\n\ -cmpd0,%0,%3\n\ +" lwsync\n\ +1: lwarx %0,0,%2 # __cmpxchg_u32\n\ +cmpw0,%0,%3\n\ bne-2f\n\ stwcx. %4,0,%2\n\ -bne-1b" -ISYNC_ON_SMP -"\n\ +bne-1b\n\ +isync\n\ 2:" : "=&r" (current_value), "=m" (*v) : "r" (v), "r" (match_value), "r" (new_value), "m" (*v) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 38 of 39] IB/ipath - More changes to support InfiniPath on PowerPC 970 systems
Hi, > Please fix the generic code if it doesn't provide the facility > you need at the moment. Don't shoe horn it into your driver > just to make up for that. Ive had 3 drivers asking for write combining recently so I agree this is a good idea. How about ioremap_wc as suggested by Willy: http://marc.theaimsgroup.com/?l=linux-kernel&m=114374741828040&w=2 Anton ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Fix some suspicious ppc64 code in dapl
Hi, I was reading through the ppc64 specific code in dapl/ and noticed some suspicious inline assembly. - EIEIO_ON_SMP and ISYNC_ON_SMP are in kernel UP build optimisations, we shouldnt export them to userspace. Replace it with lwsync and isync. - The comment says its implemenenting cmpxchg64 but in fact its implementing cmpxchg32. Fix the comment. Index: dapl/udapl/linux/dapl_osd.h === --- dapl/udapl/linux/dapl_osd.h (revision 7621) +++ dapl/udapl/linux/dapl_osd.h (working copy) @@ -238,14 +238,13 @@ #endif /* __ia64__ */ #elif defined(__PPC64__) __asm__ __volatile__ ( -EIEIO_ON_SMP -"1: lwarx %0,0,%2 # __cmpxchg_u64\n\ -cmpd0,%0,%3\n\ +" lwsync\n\ +1: lwarx %0,0,%2 # __cmpxchg_u32\n\ +cmpw0,%0,%3\n\ bne-2f\n\ stwcx. %4,0,%2\n\ -bne-1b" -ISYNC_ON_SMP -"\n\ +bne-1b\n\ +isync\n\ 2:" : "=&r" (current_value), "=m" (*v) : "r" (v), "r" (match_value), "r" (new_value), "m" (*v) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] Fix ipathverbs compile
Hi, > > The issue is that I changed the development libibverbs tree in svn to > > no longer use libsysfs, and libehca and libipathverbs are not updated > > to the new interface yet. So it is true that they won't compile > > against the development libibverbs tree without including the libsysfs > > header, but it's also true that just adding the header so they compile > > will lead to a driver library that doesn't work anyway. > > > > So I think it's better to leave them not compiling until they are > > really fixed up. > > We're in the middle of getting a new software release done here, and > just haven't had the bandwidth to look at this yet. I'll get around to > it hopefully by the middle of next week and do the appropriate updates > from the libipathverbs end. Thanks for the explanation, makes sense :) Anton ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] [PATCH] Fix ipathverbs compile
Similar to libehca, I had to add a sysfs include to be able to compile it. Am I missing something or is this correct? Anton --- Index: src/ipathverbs.c === --- src/ipathverbs.c(revision 7621) +++ src/ipathverbs.c(working copy) @@ -41,6 +41,7 @@ #include #include #include +#include #include "ipathverbs.h" ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] [PATCH] Fix some compile issues with libehca
Hi, Heres a patch to fix some warnings about missing prototypes (memset etc), and one compile error due to libsysfs not being included. This was also giving a warning when built as 32bit: my_cq->ipz_queue.queue = (u8*)resp.ipz_queue.queue; src/ehca_umain.c:239: warning: cast to pointer from integer of different size So cast it to a long first. Is that code correct for 32bit? Anton --- Index: src/ehca_uinit.c === --- src/ehca_uinit.c(revision 7621) +++ src/ehca_uinit.c(working copy) @@ -44,6 +44,7 @@ #include #include +#include #include #include #include @@ -51,6 +52,7 @@ #include #include #include +#include #include "ehca_uclasses.h" Index: src/ehca_umain.c === --- src/ehca_umain.c(revision 7621) +++ src/ehca_umain.c(working copy) @@ -53,6 +53,7 @@ #include #include #include +#include #include #include @@ -234,8 +235,8 @@ /* copy data returned from kernel */ my_cq->cq_number = resp.cq_number; my_cq->token = resp.token; - my_cq->ipz_queue.queue = (u8*)resp.ipz_queue.queue; - my_cq->ipz_queue.current_q_addr = (u8*)resp.ipz_queue.queue; + my_cq->ipz_queue.queue = (u8*)(long)resp.ipz_queue.queue; + my_cq->ipz_queue.current_q_addr = (u8*)(long)resp.ipz_queue.queue; my_cq->ipz_queue.qe_size = resp.ipz_queue.qe_size; my_cq->ipz_queue.act_nr_of_sg = resp.ipz_queue.act_nr_of_sg; my_cq->ipz_queue.queue_length = resp.ipz_queue.queue_length; @@ -321,16 +322,16 @@ my_qp->qkey = resp.qkey; my_qp->real_qp_num = resp.real_qp_num; /* rqueue properties */ - my_qp->ipz_rqueue.queue = (u8*)resp.ipz_rqueue.queue; - my_qp->ipz_rqueue.current_q_addr = (u8*)resp.ipz_rqueue.queue; + my_qp->ipz_rqueue.queue = (u8*)(long)resp.ipz_rqueue.queue; + my_qp->ipz_rqueue.current_q_addr = (u8*)(long)resp.ipz_rqueue.queue; my_qp->ipz_rqueue.qe_size = resp.ipz_rqueue.qe_size; my_qp->ipz_rqueue.act_nr_of_sg = resp.ipz_rqueue.act_nr_of_sg; my_qp->ipz_rqueue.queue_length = resp.ipz_rqueue.queue_length; my_qp->ipz_rqueue.pagesize = resp.ipz_rqueue.pagesize; my_qp->ipz_rqueue.toggle_state = resp.ipz_rqueue.toggle_state; /* squeue properties */ - my_qp->ipz_squeue.queue = (u8*)resp.ipz_squeue.queue; - my_qp->ipz_squeue.current_q_addr = (u8*)resp.ipz_squeue.queue; + my_qp->ipz_squeue.queue = (u8*)(long)resp.ipz_squeue.queue; + my_qp->ipz_squeue.current_q_addr = (u8*)(long)resp.ipz_squeue.queue; my_qp->ipz_squeue.qe_size = resp.ipz_squeue.qe_size; my_qp->ipz_squeue.act_nr_of_sg = resp.ipz_squeue.act_nr_of_sg; my_qp->ipz_squeue.queue_length = resp.ipz_squeue.queue_length; ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 21/22] ehca main file
Hi, > What is ehca_show_flightrecorder() trying to do that snprintf() is > not fast enough? If you need to pass a binary structure back to > userspace (with a kernel address in it??) then sysfs is not the right > place to put it. Look at debugfs; or relayfs might make the most > sense for your flightrecorder stuff. I agree debugfs or relayfs would be better suited. Of course as the driver matures this form of debug is probably not required at all. > +#include "hcp_sense.h" /* TODO: later via hipz_* header file */ > +#include "hcp_if.h" /* TODO: later via hipz_* header file */ I count 88 TODOs in the driver, it would be nice to get rid of some of them like the two above, so we can concentrate on the important TODOs :) > +#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,12) > +#define EHCA_RESOURCE_ATTR_H(name) \ > +static ssize_t ehca_show_##name(struct device *dev, \ > + struct device_attribute *attr,\ > + char *buf) > +#else > +#define EHCA_RESOURCE_ATTR_H(name) \ > +static ssize_t ehca_show_##name(struct device *dev, \ > + char *buf) > +#endif No need for kernel version ifdefs. Anton ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 07/22] Hypercall definitions
Hi, > Do these defines belong in the ehca driver, or should they be put > somewhere in generic hypercall support? Agreed, I think they should go into include/asm-powerpc/hvcall.h Anton ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 03/22] pHype specific stuff
Hi,
> +inline static u32 getLongBusyTimeSecs(int longBusyRetCode)
> +{
> + switch (longBusyRetCode) {
> + case H_LongBusyOrder1msec:
> + return 1;
> + case H_LongBusyOrder10msec:
> + return 10;
> + case H_LongBusyOrder100msec:
> + return 100;
> + case H_LongBusyOrder1sec:
> + return 1000;
> + case H_LongBusyOrder10sec:
> + return 1;
> + case H_LongBusyOrder100sec:
> + return 10;
> + default:
> + return 1;
> + } /* eof switch */
> +}
Since this actually returns milliseconds it might be worth making it
obvious in the function name. Also no need to use studly caps for the
function name and variable. We will fix the studly caps H_LongBusy*
stuff another day :)
> +inline static long plpar_hcall_7arg_7ret(unsigned long opcode,
> +inline static long plpar_hcall_9arg_9ret(unsigned long opcode,
These belong in arch/powerpc/platforms/pseries/hvCall.S
Anton
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 01/22] Add powerpc-specific clear_cacheline(), which just compiles to "dcbz".
Hi, > This is horribly non-portable. How much of a performance difference > does it make? How does it do on ppc64 systems where the cacheline > size is not 32? Yes, if anything we should catch cacheline aligned, multiple cacheline sized zeroing in memset. Anton ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
