Re: [patch 1/3] S390-HWBKPT: Prepare s390 for hardware breakpoint infrastructure.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Dec 14, 2009 at 07:27:24PM +0530, Mahesh Salgaonkar wrote:
> Prepare the s390 code for HW Breakpoint infrastructure patches by including
> relevant constant definitions and function declarations.

Why split this? Especially it would be nice to have function declarations and
the functions in the same patch :)

> Index: linux-2.6-tip/arch/s390/include/asm/hw_breakpoint.h
> ===================================================================
> --- /dev/null
> +++ linux-2.6-tip/arch/s390/include/asm/hw_breakpoint.h
> @@ -0,0 +1,65 @@
> + */
> +#ifndef	_S390_HW_BREAKPOINT_H
> +#define	_S390_HW_BREAKPOINT_H
> +
> +#ifdef	__KERNEL__

This header isn't exported, so the ifdef seems to be unnecessary.

> +#define	__ARCH_HW_BREAKPOINT_H

Seems to be unnecessary as well.

> +extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
> +				     unsigned long val, void *data);
> +
> +int arch_install_hw_breakpoint(struct perf_event *bp);
> +void arch_uninstall_hw_breakpoint(struct perf_event *bp);
> +void hw_breakpoint_pmu_read(struct perf_event *bp);
> +void hw_breakpoint_pmu_unthrottle(struct perf_event *bp);
> +
> +extern void arch_fill_perf_breakpoint(struct perf_event *bp);

At least this function doesn't exist in your second patch.

> Index: linux-2.6-tip/kernel/hw_breakpoint.c
> ===================================================================
> --- linux-2.6-tip.orig/kernel/hw_breakpoint.c
> +++ linux-2.6-tip/kernel/hw_breakpoint.c
> @@ -30,6 +30,9 @@
>   * This file contains the arch-independent routines.
>   */
> 
> +#ifdef CONFIG_S390
> +#include <asm/bitsperlong.h>
> +#endif

Please don't add CONFIG_S390 ifdefs in common code, unless it cannot be
avoided. Why is this needed?

>  #include <linux/irqflags.h>
>  #include <linux/kallsyms.h>
>  #include <linux/notifier.h>
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux