Re: [PATCH 01/13] [RFC] ipath basic headers



Hi Roland,

On 12/17/05, Roland Dreier <rolandd@xxxxxxxxx> wrote:
> +/*
> + * This file contains defines, structures, etc. that are used
> + * to communicate between kernel and user code.
> + */
> +
> +#ifdef __KERNEL__
> +#include <linux/ioctl.h>
> +#include <linux/uio.h>
> +#include <asm/atomic.h>
> +#else /* !__KERNEL__; user mode */
> +#include <sys/ioctl.h>
> +#include <sys/uio.h>
> +#include <sys/types.h>
> +#include <stdint.h>
> +
> +/* these aren't implemented for user mode, which is OK until we multi-thread */
> +typedef struct _atomic {
> + uint32_t counter;
> +} atomic_t; /* no atomic_t type in user-land */
> +#define atomic_set(a,v) ((a)->counter = (v))
> +#define atomic_inc_return(a) (++(a)->counter)
> +#define likely(x) (x)
> +#define unlikely(x) (x)
> +
> +#define yield() sched_yield()
> +
> +/*
> + * too horrible to try and use the kernel get_cycles() or equivalent,
> + * so define and inline it here
> + */
> +
> +#if !defined(rdtscll)
> +#if defined(__x86_64) || defined(__i386)
> +#define rdtscll(v) do {uint32_t a,d;asm volatile("rdtsc" : "=a" (a), "=d" (d)); \
> + (v) = ((uint64_t)a) | (((uint64_t)d)<<32); \
> +} while(0)
> +#else
> +#error "No cycle counter routine implemented yet for this platform"
> +#endif
> +#endif /* !defined(rdtscll) */

Do we really need this ugly userspace emulation code in the kernel?

> +/*
> + * this is used for very short copies, usually 1 - 8 bytes,
> + * *NEVER* to the PIO buffers!!!!!!! use ipath_dwordcpy for longer
> + * copies, or any copy to the PIO buffers. Works for 32 and 64 bit
> + * gcc and pathcc
> + */
> +static __inline__ void ipath_shortcopy(void *dest, void *src, uint32_t cnt)
> +{
> + void *ssv, *dsv;
> + uint32_t csv;
> + __asm__ __volatile__("cld\n\trep\n\tmovsb":"=&c"(csv), "=&D"(dsv),
> + "=&S"(ssv)
> + :"0"(cnt), "1"(dest), "2"(src)
> + :"memory");
> +}
> +
> +/*
> + * optimized word copy; good for rev C and later opterons. Among the best for
> + * short copies, and does as well or slightly better than the optimizization
> + * guide copies 6 and 8 at 2KB.
> + */
> +void ipath_dwordcpy(uint32_t * dest, uint32_t * src, uint32_t ndwords);

What is this used for? Why can't yo use memcpy?

> +#define round_up(v,sz) (((v) + (sz)-1) & ~((sz)-1))

Please use ALIGN().

> +/* These are used in the driver, don't use them elsewhere */
> +#define _IPATH_SIMFUNC_IOCTL_LOW 1
> +#define _IPATH_SIMFUNC_IOCTL_HIGH 7
> +
> +/*
> + * These tell the driver which ioctl's belong to the diags interface.
> + * As above, don't use them elsewhere.
> + */
> +#define _IPATH_DIAG_IOCTL_LOW 100
> +#define _IPATH_DIAG_IOCTL_HIGH 109

[snip, snip]

You seem to be introducing loads of new ioctls. Any reason you can't
use sysfs and/or configfs?

> +/* macros for processing rcvhdrq entries */
> +#define ips_get_hdr_err_flags(StartOfBuffer) *(((uint32_t *)(StartOfBuffer))+1)
> +#define ips_get_index(StartOfBuffer) (((*((uint32_t *)(StartOfBuffer))) >> \
> + INFINIPATH_RHF_EGRINDEX_SHIFT) & INFINIPATH_RHF_EGRINDEX_MASK)
> +#define ips_get_rcv_type(StartOfBuffer) ((*(((uint32_t *)(StartOfBuffer))) >> \
> + INFINIPATH_RHF_RCVTYPE_SHIFT) & INFINIPATH_RHF_RCVTYPE_MASK)
> +#define ips_get_length_in_bytes(StartOfBuffer) \
> + (uint32_t)(((*(((uint32_t *)(StartOfBuffer))) >> \
> + INFINIPATH_RHF_LENGTH_SHIFT) & INFINIPATH_RHF_LENGTH_MASK) << 2)
> +#define ips_get_first_protocol_header(StartOfBuffer) (void *) \
> + ((uint32_t *)(StartOfBuffer) + 2)
> +#define ips_get_ips_header(StartOfBuffer) ((ips_message_header_typ *) \
> + ((uint32_t *)(StartOfBuffer) + 2))
> +#define ips_get_ipath_ver(ipath_header) (((ipath_header) >> INFINIPATH_I_VERS_SHIFT) \
> + & INFINIPATH_I_VERS_MASK)

Please use static inlines instead for readability.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



Relevant Pages

  • Re: New Patch Fixes 43 Flaws In OS X, Many Serious
    ... A product being a "Unix" has no security implications ... Mac OS X would then become higher ... So it is with any kernel, ...
    (comp.sys.mac.advocacy)
  • Re: Story Time
    ... In this case, the General Public License says that "you" can freely use "his" code, but that if you distribute the subsequent binaries to anyone else, you must share your changes with "them". ... the GPL encourages you to charge money for shipping your code to your distributees. ... difference ins the kernel between the two versions. ...
    (comp.os.vms)
  • Re: /etc/udev/rules.d and permissions in /dev/usb
    ... > - `udevsynthesize', which writes to all those uevent files, following ... > you can't easily unplug a disk partition; you can rely on whole disk block ... >>> I personally consider even using the kernel device numbering, %n, to be ...
    (uk.comp.os.linux)
  • Re: whats next for the linux kernel?
    ... kernel code is orders of magnitude ... And multi-core processors aren't really new technology - there ... send the line "unsubscribe linux-kernel" in ...
    (Linux-Kernel)
  • Re: Micro$haft desperate! Buy Windoze 7, get (virtual) XP free!
    ... Because device drivers in operating systems with monolithic kernels, and in many operating systems with hybrid kernels, execute within the operating system kernel, it is possible to run the kernel as a 32-bit process while still supporting 64-bit user processes. ... Leopard with no trouble. ... not having 64-bit Carbon when the apps that want all that memory use Carbon. ...
    (comp.sys.mac.advocacy)