Messages in this thread Patch in this message | | | Date | Thu, 6 Jul 2006 10:52:23 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH 5/6] FDPIC: Add coredump capability for the ELF-FDPIC binfmt [try #3] |
| |
On Thu, 06 Jul 2006 13:47:27 +0100 David Howells <dhowells@redhat.com> wrote:
> From: David Howells <dhowells@redhat.com> > > Add coredump capability for the ELF-FDPIC binfmt. > > .. > > +static int dump_seek(struct file *file, off_t off) > +{ > + if (file->f_op->llseek) { > + if (file->f_op->llseek(file, off, 0) != off) > + return 0; > + } else { > + file->f_pos = off; > + } > + return 1; > +}
llseek takes a loff_t and file->f_pos is loff_t. I guess it's a bit moot on such a CPU. Was it deliberate?
(how come the kernel doesn't have a SEEK_SET #define?)
> +/* > + * Decide whether a segment is worth dumping; default is yes to be > + * sure (missing info is worse than too much; etc). > + * Personally I'd include everything, and use the coredump limit... > + * > + * I think we should skip something. But I am not sure how. H.J. > + */ > +static inline int maydump(struct vm_area_struct *vma) > +{ > + /* Do not dump I/O mapped devices or special mappings */ > + if (vma->vm_flags & (VM_IO | VM_RESERVED)) { > + kdcore("%08lx: %08lx: no (IO)", vma->vm_start, vma->vm_flags); > + return 0; > + } > + > + /* If we may not read the contents, don't allow us to dump > + * them either. "dump_write()" can't handle it anyway. > + */ > + if (!(vma->vm_flags & VM_READ)) { > + kdcore("%08lx: %08lx: no (!read)", vma->vm_start, vma->vm_flags); > + return 0; > + } > + > + /* Dump shared memory only if mapped from an anonymous file. */ > + if (vma->vm_flags & VM_SHARED) { > + if (vma->vm_file->f_dentry->d_inode->i_nlink == 0) { > + kdcore("%08lx: %08lx: no (share)", vma->vm_start, vma->vm_flags); > + return 1; > + } > + > + kdcore("%08lx: %08lx: no (share)", vma->vm_start, vma->vm_flags); > + return 0; > + } > + > +#ifdef CONFIG_MMU > + /* If it hasn't been written to, don't write it out */ > + if (!vma->anon_vma) { > + kdcore("%08lx: %08lx: no (!anon)", vma->vm_start, vma->vm_flags); > + return 0; > + } > +#endif > + > + kdcore("%08lx: %08lx: yes", vma->vm_start, vma->vm_flags); > + return 1; > +}
Three callsites - seems too large to inline.
> +#define roundup(x, y) ((((x) + ((y) - 1)) / (y)) * (y))
The GFS2 tree has
--- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -32,6 +32,7 @@ #define STACK_MAGIC 0xdeadbeef #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0])) #define ALIGN(x,a) (((x)+(a)-1)&~((a)-1)) +#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d)) #define FIELD_SIZEOF(t, f) (sizeof(((t*)0)->f)) #define KERN_EMERG "<0>" /* system is unusable */ Which seems reasonable to me. I'll steal it from them.
> +/* #define DEBUG */ > + > +#define DUMP_WRITE(addr, nr) \ > + do { if (!dump_write(file, (addr), (nr))) return 0; } while(0) > +#define DUMP_SEEK(off) \ > + do { if (!dump_seek(file, (off))) return 0; } while(0) > > ... > > +#undef DUMP_WRITE > +#undef DUMP_SEEK > + > +#define DUMP_WRITE(addr, nr) \ > + if ((size += (nr)) > limit || !dump_write(file, (addr), (nr))) \ > + goto end_coredump; > +#define DUMP_SEEK(off) \ > + if (!dump_seek(file, (off))) \ > + goto end_coredump;
Embedding returns and gotos in macros is evil. For new code it's worth doing it vaguely tastefully.
ret = dump_write(...); if (ret < 0) goto actually_return_an_error_code; > + for (vml = current->mm->context.vmlist; vml; vml = vml->next) > + segs++;
Does this need locking?
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |